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

feat(NavigationHeader): implement NavigationHeader component #885

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

codemonkey800
Copy link
Contributor

@codemonkey800 codemonkey800 commented Oct 30, 2024

Summary

Github issue: #816

Implements the NavigationHeader component for desktop and mobile

Checklist

  • Default Story in Storybook
  • LivePreview Story in Storybook
  • Test Story in Storybook
  • Tests written
  • Variables from defaultTheme.ts used wherever possible
  • If updating an existing component, depreciate flag has been used where necessary
  • Chromatic build verified by @chanzuckerberg/sds-design

@codemonkey800 codemonkey800 changed the title feat(NavigationBar): implement NavigationBar component feat(NavigationHeader): implement NavigationHeader component Oct 30, 2024
@kev-zunshiwang
Copy link

kev-zunshiwang commented Oct 30, 2024

Looking great, thanks Jeremy!!

Desktop Feedback

  • Logo Slot
    • Update color to Base/border for the dash border line around "logo slot"
    • Unbold the text "logo slot"
    • The entire area of "logo slot / logo name / beta tag" should be linked to the home page of the product. Not sure if this affects the code
  • Tag
    • Update text inside the tags to title case
    • Remove the hover effect for the tags
  • Buttons
    • Update the "person" minimal button to a icon button (see design for the specific spec)
  • Shadow
    • I'm not sure if you implemented a shadow into the component, if you did please remove it.

@kev-zunshiwang
Copy link

Mobile Feedback
Logo

  • whole area should be a link

Secondary Nav Item

  • the whole row should be a target area - see spec for details
  • On hover, the background color of the whole row should have a gray (base/surface-secondary)
    Secondary Nav Sub Item
  • the whole row should be a target area - see spec for details
  • On hover, bold the text with Narrow/fontBodyS/600 in addition to the background color change.

Buttons

  • the primary / secondary buttons / minimal button should extend to the whole row - see spec for detals
  • Use the S size of the person icon, the illustration is different from the larger size
  • bold the text - Narrow/fontBodyM/600

Base automatically changed from jeremy/mobile-font-styles to main November 8, 2024 23:49
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.

3 participants