update to match exposed uswds pageheader #20
Merged
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-next-test ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
NOTE: Still need to build library package and update version to see preview.... |
4 tasks
AliceR
reviewed
Nov 26, 2024
app/components/nav.tsx
Outdated
| </div> | ||
| </nav> | ||
| </div> | ||
| <nav id='nav'> |
There was a problem hiding this comment.
I am wondering if this nav element isn't conflicting with the one created by the header component? What is it's purpose here?
There was a problem hiding this comment.
https://developer.mozilla.org/en-US/docs/Web/HTML/Element/nav - it came with the template nextJs setup, could probably just remove it
There was a problem hiding this comment.
I think we should remove the unnecessary elements, <nav> and <aside>, as we have those implemented with the <PageHeader> component itself.
sandrahoang686
added a commit
to NASA-IMPACT/veda-ui
that referenced
this pull request
Dec 9, 2024
**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 - [ ] Validate the header contents - [ ] Use the navigation to access different pages - [ ] Use keyboard navigation, making sure the navigation is accessible - [ ] Instances can use the header and customize its content - [x] logo - [x] heading - [x] primary nav items - [x] secondary nav items
dzole0311
added a commit
to NASA-IMPACT/veda-ui
that referenced
this pull request
Dec 10, 2024
…build (#1275) **Related Ticket:** #1191 **Related Next.js PR:** developmentseed/next-veda-ui#20 ### Description of Changes - Bundle USWDS styles and custom styles with our components - Fix USWDS asset paths to work in Next.js via PostCSS URL handling - Rename SCSS files to match component names to prevent namespace conflicts (otherwise they throw `namespace exists` errors when they are imported into one entry SCSS file) - Consolidate styles into a single entry point (`styles.scss`) - Update `yarn.lock` / resolve merge conflicts ### Main problem this PR addresses When our USWDS-styled components were used in Next.js, they appeared unstyled because USWDS assets weren't properly bundled. This PR: 1. Bundles all needed USWDS assets with our components 2. Resolves USWDS asset paths correctly in Next.js 3. Makes styles work automatically in Next.js without the need for manual style imports The third point is interesting because Next.js detects and loads any CSS dependencies that Parcel bundled with our components, which I hope makes the developer experience much smoother. This means we could do this: ``` // Just import the component, styles come automatically once the VEDA UI lib is used import { Header } from '@veda-ui/lib' ``` And the component will be styled with the VEDA UI styles, instead of having to: ``` // Had to manually import styles in Next.js app import '@veda-ui/lib/styles.css' import { Header } from '@veda-ui/lib' ``` ### Notes & Questions About Changes Few things to note (cc @hanbyul-here @sandrahoang686 @AliceR) 1. This approach only includes USWDS styles that we're actively using in our components. These styles are managed through our main `styles.scss` file. This means we're keeping things lightweight by not including styles for USWDS components we don't use (like the stepper component). If a Next.js project needs additional USWDS components later, we can easily add them to `styles.scss` or they could install the USWDS library and get access to it. This keeps our bundle size small and efficient (we only ship what we actually need for VEDA) 2. Style organization: - Main entry point: `app/scripts/styles/styles.scss` - Component styles should be imported in this entry file, not in the components. However, they can be kept in the respective component directory for easier look up - Next.js apps don't need to install USWDS as a dependency, everything comes with our components ### Validation / Testing VEDA UI 1. Check preview deployment 2. Check that USWDS styles work (page header) 3. Check that custom styles work (page header) 4. Confirm icons display correctly (nav chevrons) Next.js 1. Test locally using [symlink](https://github.com/NASA-IMPACT/veda-ui/blob/main/docs/development/REGISTRY.md) with veda-ui branch (`1137-implement-new-ds-page-header`, now merged to `main`) 2. Check that both USWDS and custom styles work (page header) 3. Check that no manual style imports are needed
|
Note, we need to still add the About page and the contact us modal. I added this as a todo item here: NASA-IMPACT/veda-ui#1282 |
AliceR
approved these changes
Dec 10, 2024
app/components/nav.tsx
Outdated
| </div> | ||
| </nav> | ||
| </div> | ||
| <nav id='nav'> |
There was a problem hiding this comment.
I think we should remove the unnecessary elements, <nav> and <aside>, as we have those implemented with the <PageHeader> component itself.
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.
Available PR templates