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

Breadcrumbs: Don't add breadcrumb for the current tab #68230

Merged
merged 1 commit into from
May 11, 2023

Conversation

torkelo
Copy link
Member

@torkelo torkelo commented May 10, 2023

Not sure why I added this, it is useful for pages with many tabs to find your way back to the "home" / default tab. But with scene drilldowns creating deep breadcrumbs, anything we can do to limit the number is good. It's also not great for mobile views because if we show the current tab, we have to have 3 breadcrumbs to find your way back to your the parent "page".

Before:
image

After:
Screenshot from 2023-05-10 16-25-29

@torkelo torkelo requested a review from a team as a code owner May 10, 2023 14:27
@torkelo torkelo requested review from ashharrison90 and eledobleefe and removed request for a team May 10, 2023 14:27
@torkelo torkelo added this to the 10.0.0 milestone May 10, 2023
@torkelo torkelo added product-approved Pull requests that are approved by product/managers and are allowed to be backported backport v10.0.x labels May 10, 2023
@torkelo torkelo changed the title Breadcrumbs: Don't add breadcrumbs for the current tab Breadcrumbs: Don't add breadcrumb for the current tab May 10, 2023
@torkelo torkelo requested a review from a team May 10, 2023 16:16
Copy link
Contributor

@ashharrison90 ashharrison90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@torkelo have to confess i also can't really remember the problem this was supposed to fix 😅

have looked through the old PR that added it (#61835) and had a play around locally and from what i can tell the changes seem fine.

maybe it would be safer if this doesn't get backported though. would give us a lot more time to spot any potential problems? up2u 🤔

@torkelo torkelo modified the milestones: 10.0.0, 10.1.x May 11, 2023
@torkelo torkelo added no-backport Skip backport of PR and removed product-approved Pull requests that are approved by product/managers and are allowed to be backported backport v10.0.x labels May 11, 2023
@torkelo torkelo merged commit c9ce1a2 into main May 11, 2023
17 of 19 checks passed
@torkelo torkelo deleted the no-breadcrumbs-for-tabs branch May 11, 2023 13:16
ryantxu pushed a commit that referenced this pull request May 11, 2023
Breadcrumbs: Don't add breadcrumbs for the current tab
ryantxu pushed a commit that referenced this pull request May 16, 2023
Breadcrumbs: Don't add breadcrumbs for the current tab
@ricky-undeadcoders ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023
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.

None yet

4 participants