codefordenver / encorelink

Connecting musician volunteers with healthcare centers
https://encorelink.herokuapp.com/
ISC License
20 stars 14 forks source link

Terms checkbox #389

Closed jsullivan5 closed 6 years ago

jsullivan5 commented 6 years ago

This PR closes #385 #389

What does this PR do?

How does this PR make you feel? 😀 😀 😀 😀 😀 😀 😀 😀

eric-yates commented 6 years ago

I'd personally put the text label on the right side of the checkbox. That is consistent with typical formatting guidelines. Also, could you add an X to close the terms pop-up window?

I'm not knowledgeable enough to know whether the changes you made are well done. I'll assign Nathan to review

jsullivan5 commented 6 years ago

@eric-yates Good call on the checkbox and label. That has been fixed. Did you see the close button at the bottom of the terms? I can get an X style close button in the top right if we want. I feel like users are used to scrolling to the bottom of these to close them though.

eric-yates commented 6 years ago

@jsullivan5 Looks good on the checkbox. I did not see the close button at the bottom, so disregard the X close button. It'd be best to have people scrolling to the bottom to close anyways. Thanks and great work!