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

AppSideNav - component (HDS-3800) #2384

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

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented Sep 3, 2024

STATUS:


📌 Summary

If merged, this PR will add the AppSideNav component to HDS.

Previews:

Related branches:

🛠️ Detailed description

The majority of code was duplicated from the SideNav and then updated to implement the new AppSideNav component.

Major differences of note:

  • The AppSideNav has an updated visual design using lighter colors than the SideNav
    • Dark theme styles are no longer added to nested Button & Dropdown components
  • The AppSideNav :header named block was simplified to act as a basic yielded container
    • The :logo & :actions child named blocks were removed
    • The Header and Header::HomeLink child components were removed.
  • The Navigator Narrator (skip link) was removed as the AppSideNav is meant to always be paired with the AppHeader which already includes this

AppSideNav Follow-ons (recommended):

🔗 External links


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Sep 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Sep 20, 2024 9:26pm
hds-website ✅ Ready (Inspect) Visit Preview Sep 20, 2024 9:26pm

@KristinLBradley
Copy link
Contributor Author

@alex-ju I pushed up fixes for issues. For some items I would like to look at them as a follow on if that's ok. (Listed below.)

AppSideNav Follow-ons:

@KristinLBradley
Copy link
Contributor Author

KristinLBradley commented Sep 20, 2024

There's a test failing I will investigate...

Looks like just a timeout error. restarted test.

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

I think this is good to go, short of one note about the list item color that should probably be a quick fix. Let me know what you think or any other questions!

KB: Updated the color, thanks for reviewing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants