-
Notifications
You must be signed in to change notification settings - Fork 26
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(topnav): update top navigation component specs #807
Conversation
Deploy preview for helix-ui ready! Built with commit 54c1851 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danielmdesigns please see Top Navigation updates based on current specs:
https://deploy-preview-807--helix-ui.netlify.app/components/topnav
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is some initial feedback:
- Dropshadow for dropdowns are too dark. I would suggest reducing the opacity/alpha transparency there. Selecting this object in Abstract will give you the box-shadow specs that we used in our design
- Nav links should not be in all caps
- Default link colors are wrong. We should use #757575 for all Top & Bottom nav links
- Remove background color from hover. Hover/Selected state link color will be #333333
- Default Icon color should be #9E9E9E (hover/selected icons will be #333333)
- Dropdown links should not have a background on hover. The Default link color will be #333333 with a Hover/Selected color of #970C8D
- Button hover state color is wrong. Button states should be Blue-500 Default, Blue-700 Hover, Blue-900 Selected
- Button should not have border-color on hover
The Navigation link in Abstract has been updated. Please reference this to support the feedback provided above. If you have any other questions, please do not hesitate to ask.
@danielmdesigns : Please review the changes based on your feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Thanks for addressing those previous concerns. I believe once the icons are updated (James & I just talked about these) then I believe this will be ready to approve. Great work, thanks for the quick turn around on the updates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please see changes below, and squash commits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Description
<hx-topnav>
implementing color palette 2.0 stylesWhat are the relevant story cards/tickets? Any additional PRs or other references?
Jira: SURF 2142
Before you request a review for this PR:
yarn test
to ensure all tests passed?