Refactor page header to use USWDS components #1247
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
✅ Deploy Preview for veda-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
AliceR
changed the title
sandrahoang686
changed the title
@use is for importing and using styles within the current file, while @forward is for re-exporting styles to make them available to other files that import the current file. I am still not sure, how to decide when to use @use and when to use @forward in this project. The design system documentation only mentions @forward, so I am going with this.
I was not able to set up the formatting to be run via the eslint extension, so I use the prettier extension for scss files - see settings.json.sample for a suggestion on how to configure the vscode workspace
this requires more refactoring, to use only USWDS eventually
the refactored uswds header now lives in a separate directory
as the sliding header did not properly work with the USWDS components
…tton from google-form, update nav configs
not sure why the inline comment was ending up in the variable as part of the string, but a separate line for the comment fixed it
Starting with the nav dropdown button
make other dropdowns close on toggle
to improve readability
not sure about this... feels even more confusing.
…p for uswds tokens as sass vars
wip, the test does not run yet...
…iner version condition, port pageheader to fn comp and optimize
dzole0311
reviewed
Nov 26, 2024
dzole0311
reviewed
Nov 27, 2024
There was a problem hiding this comment.
Thanks for the great work @sandrahoang686 @AliceR! I did a first pass through the code / functionality and only have few nit comments below which should be easily addressable. Let me know what you think.
app/scripts/components/common/page-header/nav/create-dynamic-nav-menu-list.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/common/page-header/nav/nav-dropdown-button.tsx
Outdated
Show resolved
Hide resolved
app/scripts/components/common/page-header/nav/create-dynamic-nav-menu-list.tsx
Show resolved
Hide resolved
and also noting that we should really fix those pre-commit hooks :D
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related Ticket: Close #1137
Related PR: NextJs PR developmentseed/next-veda-ui#20
Description of Changes
This change implements the page header with the USWDS ui components
Notes & Questions About Changes
The new header will have some design inconsistencies with the rest of the page, but that is intentional as we go through the revamp of the ui.
Validation / Testing
- [x] logo
- [x] heading
- [x] primary nav items
- [x] secondary nav items