refactor: Refactor page header to use USWDS components #1247
Conversation
✅ 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...
| <React.Fragment key={item.id}> | ||
| {item.actionId === 'open-google-form' && ( | ||
| <> | ||
| <a className='usa-nav__link' id={item.id}> |
There was a problem hiding this comment.
without href, a tag doesn't get keyboard focus (And its hover style doesn't show the cursor.) Can we give some dummy href to this a tag (or give a tabindex, mark the a tag as a button (role="button") and move the onclick event to a tag?
There was a problem hiding this comment.
good callout, this should actually be a button since its a CTA. Now works as expected, when tabbing to CTA and clicking enter it opens modal.
| } | ||
|
|
||
| .usa-nav__secondary-item .usa-nav__link { | ||
| padding: 0; |
There was a problem hiding this comment.
I thought uswds has enough utility classes that we don't need these individual styles for each item - https://designsystem.digital.gov/utilities/margin-and-padding/ any reason not using these utility classes?
There was a problem hiding this comment.
yeah, if you are talking about doing something like ...
<USWDSExtendedNav
primaryItems={primaryItems}
secondaryItems={secondaryItems}
mobileExpanded={expanded}
onToggleMobileNav={toggleExpansion}
className='padding-0'
/>
unfortunately it doesn't work because this component has nested elements and this wants to just target the secondary nav items, not the primary. If you know how to or mean something else though, please suggest! Otherwise this follows this pattern more closely.
app/scripts/components/common/page-header/logo-container/styles.scss
Outdated
Show resolved
Hide resolved
sandrahoang686
changed the title
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