Skip to content
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

fix(breadcrumb): prevent wrapping to second line #8789

Closed
wants to merge 2 commits into from

Conversation

tay1orjones
Copy link
Member

@tay1orjones tay1orjones commented May 27, 2021

Closes #8779

Prevent breadcrumbs from wrapping to a second line. I do have some reservations on this change request and would like some feedback:

The initial issue points this out, but the breadcrumb guidance on the webstie states

Breadcrumbs should never wrap onto a second line.

@carbon-design-system/design How literally should we interpret this? I think the intent is that when breadcrumbs would be too long and begin wrapping, consumers should instead switch to the "overflow menu mode". I'm not sure if we should force breadcrumbs to stay on a single line like this and use overflow: hidden to ensure they don't resize when they hit the browser edge on smaller screen sizes.

Changelog

Changed

  • breadcrumb: prevent items from wrapping to second line

Testing / Reviewing

Breadcrumb items should not wrap at small screen sizes. Try adding double/triple the amount of breadcrumb item li's and see how it clips the ones that are beyond the browser's edge.

hiding.breadcrumbs.mov

@tay1orjones tay1orjones requested a review from a team as a code owner May 27, 2021 21:22
@tay1orjones tay1orjones requested review from emyarod and jnm2377 May 27, 2021 21:22
@emyarod emyarod requested review from a team and alisonjoseph and removed request for a team May 27, 2021 21:23
@emyarod emyarod requested review from a team and thyhmdo and removed request for alisonjoseph and a team May 27, 2021 21:24
@netlify
Copy link

netlify bot commented May 27, 2021

✔️ Deploy Preview for carbon-elements ready!

🔨 Explore the source changes: 140731d

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-elements/deploys/60b0117f4b67390007a6f1d7

😎 Browse the preview: https://deploy-preview-8789--carbon-elements.netlify.app

@netlify
Copy link

netlify bot commented May 27, 2021

✔️ Deploy Preview for carbon-components-react ready!

🔨 Explore the source changes: f9812ba

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/60b00d8af1ef010007a443e6

😎 Browse the preview: https://deploy-preview-8789--carbon-components-react.netlify.app/iframe

@netlify
Copy link

netlify bot commented May 27, 2021

✔️ Deploy Preview for carbon-components-react ready!

🔨 Explore the source changes: 140731d

🔍 Inspect the deploy log: https://app.netlify.com/sites/carbon-components-react/deploys/60b0117f1d9e6e0008bae85f

😎 Browse the preview: https://deploy-preview-8789--carbon-components-react.netlify.app/iframe

@jnm2377
Copy link
Contributor

jnm2377 commented May 31, 2021

Hmm... I have some initial questions.

  • should we have guidelines as to how many breadcrumbs deep we recommend users going to (max limit)? We do this with tabs, which of course isn't the same kind of navigation, but bc of the visual style being similar... might be something that might help prevent this sort of issue.
  • if we do want to go with this hidden/scrolling behavior on smaller screens, would we want to add some sort of fade for when it overflows like the way we do with code snippet? To indicate that there's more content?
  • or...could we add that overflow menu functionality by default? I feel like I prefer this solution since it's already what we recommend.

@emyarod
Copy link
Member

emyarod commented Jun 2, 2021

bump @thyhmdo for a visual review and clarification on breadcrumb guidance!

@thyhmdo
Copy link
Member

thyhmdo commented Jun 2, 2021

@jnm2377 I thought we have a pretty clear guidance here on using overflow menu: "When space becomes limited, use an overflow menu to truncate the breadcrumbs. The first and last two page links should be shown, but the remaining breadcrumbs in between are condensed into an overflow menu. Breadcrumbs should never wrap onto a second line." Unless this doesn't work on mobile screen size.

We don't have a thought of what number could be good for breadcrumbs right now, but similar to tabs (no more than 6) would be ideal. I think if it comes to user research and testing, the product teams need to see if it's viable for their users.

@tw15egan
Copy link
Collaborator

tw15egan commented Jun 2, 2021

I feel that unless we auto-convert the Breadcrumbs to the overflow menu variant on small screen sizes, they should break into multiple lines. Auto converting to the overflow menu variant is a much larger enhancement and is likely out of scope for this sprint.

@tay1orjones
Copy link
Member Author

@tw15egan Yeah, I agree. Adding the auto-convert to overflow on small sizes would be the ideal solution to this.

I'm good with keeping the existing behavior until we can take that on, in the meantime close this PR and use the contents as a guide for how teams can override the wrapping behavior. I'll update the original issue. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Breadcrumbs] Wrapping weirdly. Not behaving according to guidance
5 participants