feat(component): Add Breadcrumb component#2930
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
00ec9d2 to
2303ca1
Compare
6710038 to
bf0f317
Compare
bf0f317 to
f5d1ec8
Compare
louismaximepiton
left a comment
There was a problem hiding this comment.
I didn't finished the review of the CSS yet, but here is a first draft of the review
louismaximepiton
left a comment
There was a problem hiding this comment.
Finished the review on my side.
I'm still convinced we could do better for the CSS by avoiding repeating the svg acall, however, as a first version, I'm fine landig it as-is.
Just small changes for me and wait for a11y review maybe ?
scss/_variables-dark.scss
Outdated
| // | ||
|
|
||
| $breadcrumb-divider-filter-dark: $invert-filter !default; // OUDS mod | ||
| // OUDS mod: no $accordion-button-icon-dark |
scss/_breadcrumb.scss
Outdated
| flex-shrink: 0; | ||
| width: $ouds-breadcrumb-size-icon; | ||
| height: $ouds-breadcrumb-size-icon; | ||
| margin-right: var(--#{$prefix}breadcrumb-item-padding-links) #{"/* rtl:ignore */"}; // OUDS mod |
There was a problem hiding this comment.
| margin-right: var(--#{$prefix}breadcrumb-item-padding-links) #{"/* rtl:ignore */"}; // OUDS mod | |
| margin-right: var(--#{$prefix}breadcrumb-item-padding-links); // OUDS mod |
| --- | ||
|
|
||
| {{< callout-soon >}} | ||
| ## Example |
There was a problem hiding this comment.
Align with other pages
| ## Example | |
| ## Basic example |
| {{< /example >}} | ||
|
|
||
|
|
||
| ## Responsive behavior |
There was a problem hiding this comment.
It's quite not the responsive behavior, but I can't find better words
There was a problem hiding this comment.
"Behaviour on smaller screens" ?
There was a problem hiding this comment.
tried something
| </ol> | ||
| </nav> | ||
|
|
||
| <nav aria-label="breadcrumb"> |
There was a problem hiding this comment.
Maybe we should find different labels for the a11y of the page ?
| Use an ordered or unordered list with linked list items to create a minimally styled breadcrumb. | ||
|
|
||
| {{< example >}} | ||
| <nav aria-label="breadcrumb-basic"> |
There was a problem hiding this comment.
aria-label must contain a string that will be read by the vocal assistant. It is not a reference to an id or another element, so I think we can remove the hyphen in each aria-label of this page to get better vocalization.
scss/_breadcrumb.scss
Outdated
| @mixin has-separator() { | ||
| padding-left: var(--#{$prefix}breadcrumb-item-padding-icon); | ||
|
|
||
| &::before { |
There was a problem hiding this comment.
As talked together, we could change for an after pseudo element, so we could display it for every item but the last, and don't have to add it only for the first shown element. It would be simpler to manage the "responsive" part and reduce duplicated generated code related to the placehoder.
Note: Please transform
- [ ]into- (NA)in the description when things are not applicableRelated issues
closes #2896
Description
Motivation & Context
Types of change
Live previews
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge