-
Notifications
You must be signed in to change notification settings - Fork 740
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
Refactor top navigation #599
Conversation
Addresses #595 1. Changed behavior in which top nav no longers pushes content below it. 2. Fixed display of secondary navigation when hovering over other primary nav tabs. 3. Updated colors.
Tested locally, though the top-nav behavior has been addressed, some of the JS in that file that was dealing with the moments UI makes me want to hold off on merging it until we have the react version up, will review as soon as that's been taken care of |
@abrophy Heck yeah! React here we come! I'll finish up a few things on this branch so that we have a finalized HTML structure / CSS classnames, and then I'll make another branch to convert these changes into React components. |
Moved to using a container for the secondary navigations and control the animation height for the container rather than for each individual secondary navigation list.
Was unnecessarily concerned about possible conflicts with react, have tested it and it looks to be working |
1. Added initial infrastructure for CSS Modules. 2. Created stub React components for Header. 3. Created logo React component
Mind if I use snippets from this commit in the documentation task on how to use React? :) Glad to see that the react infrastructure upgrade is seeing use now 👍 |
@nma Oh, it's far from done, but yeah, it'd be good to document a primer on how to build a React component and embed it into the ERB. |
@timofonic #691 Looks like we'll want to focus on that; this PR here is minor and will be obsolete by #691. |
Addresses #595