Skip to content
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

Header: The Pegasus header's user menu & hamburger no longer wrap #16984

Merged
merged 3 commits into from Aug 14, 2017

Conversation

breville
Copy link
Member

@breville breville commented Aug 10, 2017

With a particularly long set of options in the Pegasus header on a responsive page, there was a chance that the user menu and hamburger would wrap into the whitespace below, rendering them invisible, at certain browser widths.

The same thing was occurring on the teacher-dashboard, since the header has remained responsive even as those pages themselves are not.

This fix brings the Pegasus functionality closer to the dashboard, by not allowing these right-hand components to wrap. It does produce some overlapped rendering, but going forward we can tune the content of the header and the responsive rules to prevent that.

I took this opportunity to fix something that was really bugging me: the padding on the right of the header is now the same as the padding on the left.

To remain consistent, the dashboard header also gets this new right-hand padding size.

Since a change to the header will fail a lot of Eyes visual diff tests, I took the opportunity to improve the user menu's up/down glyph to use the same font-awesome arrows as the hamburger. This work exposed some layout complications in the dropdown menu and how it related to its parent button which has been made more robust.

on a responsive pegasus page

screenshot 2017-08-11 14 38 33

on a non-responsive pegasus page

screenshot 2017-08-11 14 39 21

on a dashboard page

screenshot 2017-08-11 14 39 44

dropdown on pegasus

screenshot 2017-08-11 15 16 35

dropdown on dashboard

screenshot 2017-08-11 15 16 28

With a particularly long set of options in the Pegasus header on a responsive page, there was a chance that the user menu and hamburger would wrap into the whitespace below, rendering them invisible, at certain browser widths.

The same thing was occurring on the teacher-dashboard, since he header has remained responsive even as those pages themselves are not.

This fix brings the Pegasus functionality closer to the dashboard, by not allowing these right-hand components to wrap.  It does produce some overlapped rendering, but going forward we can tune the content of the header and the responsive rules to prevent that.
Make the hamburger the same distance from the edge as it is on pegasus.  This also makes it the same distance as the logo on the left.
@@ -983,7 +983,7 @@ $default-modal-width: 640px;
.header_right {
position: absolute;
float: left;
right: 25px;
right: 16px;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think you can get rid of float: left; since it should have no effect on an absolutely positioned element.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch. I'd discovered this when messing with the DOM and then forgot when implementing here.

@breville
Copy link
Member Author

fyi @wjordan

@breville
Copy link
Member Author

PTAL

@davidsbailey
Copy link
Member

davidsbailey commented Aug 11, 2017

these look like they affect the user menu. can you include screenshots?

@breville breville merged commit c610c99 into staging Aug 14, 2017
@breville breville deleted the header-do-not-wrap-user-menu branch August 14, 2017 17:53
@davidsbailey
Copy link
Member

Hi Brendan, please ping if you're making modifications only to the github description next time, as these do not generate email updates.

wjordan added a commit that referenced this pull request Aug 25, 2017
re-apply changes from #16984
(reverted in merge conflict by #17044)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants