-
Notifications
You must be signed in to change notification settings - Fork 0
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): L3-376 Added Header component #168
Conversation
…e state across child components
# Conflicts: # src/components/Link/_link.scss
✅ Deploy Preview for phillips-seldon ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
src/components/Navigation/NavigationItem/NavigationItem.stories.tsx
Outdated
Show resolved
Hide resolved
src/components/Navigation/NavigationItemTrigger/NavigationItemTrigger.tsx
Show resolved
Hide resolved
src/components/Navigation/NavigationItemTrigger/NavigationItemTrigger.tsx
Outdated
Show resolved
Hide resolved
<button | ||
type="button" | ||
onClick={handleMenuToggle} | ||
className={classnames(`${px}-header__toggle-btn`, { |
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.
is an aria-label
needed here? or will the span be picked up as the accessible name of this button?
Do we need aria-expanded
and aria-controls
here?
https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-expanded
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.
Yes, I believe we do. To that point do we need to pass in an id
for the menu which this toggle controls?
…PhillipsAuctionHouse/seldon into feature/L3-376_Header_Component
…use spacing tokens
…PhillipsAuctionHouse/seldon into feature/L3-376_Header_Component
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.
LGTM
# [1.27.0](v1.26.2...v1.27.0) (2024-07-08) ### Features * **header:** L3-376 Added Header component ([#168](#168)) ([7e196c5](7e196c5))
🎉 This issue has been resolved in version 1.27.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Jira ticket
L3-376
Screenshots
DESKTOP
MOBILE
Summary
This PR is for the addition of the Header component to the seldon repo. The following components have been added in this PR:
Acceptance Test (how to verify the PR)
Things to look for during review
feat(scope): ...
if aminor
release should be triggered.phillips
class prefix are using the prefix variabledata-testid
attribute.New Components
index.ts
file