-
Notifications
You must be signed in to change notification settings - Fork 54
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
Breadcrumb: Add the mobile version #2003
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for boosted ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Small comments about the migration guide.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
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.
Looks good and migration guide is ok.
Kudos, SonarCloud Quality Gate passed! 0 Bugs No Coverage information |
All looks good me. Thanks |
@louismaximepiton I haven't checked this PR yet but we need to check if #520 (comment) can be useful here or not at all. I mean, is a link with chevron usable here, or would it be a completely separate component? |
We probably can use a kind of link with chevron but it means changing the markup to get some utilities ( |
Got the same feeling so let's keep this separate issue to maybe provide this kind of links if they can be used outside the context of breadcrumbs. |
There's something we need to clarify in this PR in one of our weekly sessions, the links are no longer underlined contrary to our main branch. |
ok, good to me ! |
padding-left: var(--#{$prefix}breadcrumb-item-padding-x) #{"/* rtl:ignore */"}; | ||
color: var(--#{$prefix}breadcrumb-divider-color); | ||
content: var(--#{$prefix}breadcrumb-divider, escape-svg($breadcrumb-divider)) #{"/* rtl:"} var(--#{$prefix}breadcrumb-divider, escape-svg($breadcrumb-divider-flipped)) #{"*/"}; | ||
transform: scaleX(-1) #{"/* rtl:"} scaleX(1) #{"*/"}; |
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.
Instead of transforming it automatically, I'm wondering if we shouldn't provide a $breadcrumb-divider-mobile
(not sure about the naming). Because if you choose "/" as a divider, in mobile mode you probably don't want it to be "".
Note: Please transform
- [ ]
into- (NA)
in the description when things are not applicableRelated issues
Closes #891.
Linked to #537.
Description
Add a small version of the breadcrumb.
Accepted by a11y with ux doubts, but fine from a11y pov.
Rebrand a bit the Breadcrumb so it has no underline.
From design point of view, it was asked:
Motivation & Context
The brand and help people to have breadcrumb in their sites.
Types of change
Live previews
Please check that the mobile version looks like the design.
Please compare to https://main--boosted.netlify.app/docs/components/breadcrumb to make sure the desktop version doesn't change in any case.
Checklist
Contribution
Accessibility
Design
Development
Documentation
Checklist (for Core Team only)
After the merge