-
Notifications
You must be signed in to change notification settings - Fork 81
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: header component #83 #102
Conversation
Hey @eamahanna realized I have something similar to the navigation work you are taking on in the |
Add title component
…s/react-uswds into em-create-header-component-#83
Hm I thought there was some difference in markup between the two as well? If it's just a matter of an additional CSS class then an |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should regroup on organizing the various header/footer/nav subcomponents!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a lot of more nitpicky comments now that I've had a chance to compare this markup with the USWDS markup. let me know if there's anything I can help with or needs clarification!
Additionally, nav items should be able to be indicated as "current" (so they can apply the usa-current
class). I think the class may be applied to the simple links directly in the stories, but menu items & dropdown buttons may need to accept a prop like isCurrent
in order to apply the class
@suzubara Thanks for bringing up the |
👍 ok, I'm not sure if it's relevant for the footer since I think it's less common for footer links to show a "current" class (as opposed to the header nav) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see nothing major blocking. I'm going to approve since my comments are just cleanup, trust @eamahanna to integrate what makes sense.
src/components/header/NavDropDownButton/NavDropDownButton.test.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, you took on a whole lot with this component! all of my comments are suggestions, I'm approving and leaving it up to you if you want to incorporate them :)
PR Description
This PR creates components which are necessary to create the uswds basic header described here.
Created Components:
Each of the components has associated stories and tests.
For Reviewers
This is a huge PR......my bad. Next time I will try and break up the changes better. The title component is good to go already. Please let me know if there are any organization changes. I broke up the components the best I could, but having another set of eyes on everything is good! Thank you in advance!