-
Notifications
You must be signed in to change notification settings - Fork 82
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Increase contrast for the school page sidebar. #1431
Conversation
The only thing that bugs me about this is the text wrapping of the first button. I would prefer that it read:
@nguyenist, what do you think? This would require a bit more effort to get it working correctly on different screen sizes; for instance, it might not need to wrap on mobile. cc @abisker @barkimedes for visibility |
And let me just say, once again: thank you for putting your time into this, @stevenbuccini! We're going to try to get this into our next release. |
@jjoteal has signed off on this in #1419. Merging. Thanks again, @stevenbuccini! 🚀 |
Increase contrast for the school page sidebar.
@shawnbot I agree it would look cleaner! *Also coming in with fresh eyes on the project, but wouldn't spend too much time on this unless users are missing the button / having difficulty with it because of the labeled text on the button. And yes, you're right, it seems like no wrap needed on mobile! (at least on an iPhone 6S Plus): |
@shawnbot @nguyenist My intuition says that if you just put a |
Yeah, that might do it @stevenbuccini. Although I think we'd want |
Fixes #1419.
I mucked around with the CSS a little more than I'd like. On my Macbook Air, the previous CSS eliminated the left/right padding, which didn't look good. The downside to adding the padding back is that on medium screensizes the text within the button breaks onto two lines easily.
I'd like some feedback on whether or not this padding change is necessary. Here's how it looks right now on my Macbook Air, browser full-paged: