Skip to content

[v12] feat(ui-breadcrumb): Breadcrumb_rework #2284

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.

Sign up for GitHub

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

Jump to bottom
Open
git-nandor wants to merge 2 commits into
base: v12
Choose a base branch
from
Open

[v12] feat(ui-breadcrumb): Breadcrumb_rework #2284

git-nandor wants to merge 2 commits into from

Conversation

@git-nandor
Copy link
Contributor

@git-nandor git-nandor commented Dec 2, 2025 •
edited
Loading

INSTUI-4786

Summary

Migrated Breadcrumb component to use the new theming system and updated the separator icon to use Lucide icons.
Modify separator style calculation.

Changes

  • Modified Breadcrumb to pass size prop to child BreadcrumbLink
  • Removed font-size styling from Breadcrumb styles - now handled by Link's size prop
  • Replaced old icon with ChevronRightInstUIIcon from @instructure/ui-icons-lucide
  • Added @instructure/ui-icons-lucide dependency to package.json
  • Implemented proper size mapping: Breadcrumb sizes (small/medium/large) → Icon sizes (xs/sm/md)

Simplified Layout with CSS Gap:

  • Refactored from complex absolute positioning to clean CSS gap property
  • Applied size-variant gaps (gapSm/gapMd/gapLg) to flexbox breadcrumb container
  • Removed all complex fontsize based calc() formulas and positioning logic

Test plan

On the documentation page, verify that everything displays and works correctly,
and check that it matches the Figma design.

Co-Authored-By: 🤖 Claude

@git-nandor git-nandor self-assigned this Dec 2, 2025
@github-actions
Copy link

github-actions bot commented Dec 2, 2025 •
edited
Loading

PR Preview Action v1.6.3

🚀 View preview at
https://instructure.design/pr-preview/pr-2284/

Built to branch gh-pages at 2025-12-05 17:42 UTC.
Preview will be ready when the GitHub Pages deployment is complete.

@git-nandor git-nandor force-pushed the INSTUI-4795_link_rework_v2 branch 2 times, most recently from 877adcc to c4d09d5 Compare December 3, 2025 07:52
@git-nandor git-nandor changed the base branch from INSTUI-4795_link_rework_v2 to v12 December 3, 2025 21:24
@git-nandor git-nandor changed the base branch from v12 to INSTUI-4795_link_rework_v2 December 3, 2025 21:27
@git-nandor git-nandor changed the base branch from INSTUI-4795_link_rework_v2 to v12 December 4, 2025 08:21
@git-nandor git-nandor changed the base branch from v12 to INSTUI-4795_link_rework_v2 December 4, 2025 08:22
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch 2 times, most recently from ad9782b to a644bc4 Compare December 4, 2025 08:35
@git-nandor git-nandor marked this pull request as ready for review December 4, 2025 08:55
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from a644bc4 to e9f0e19 Compare December 4, 2025 09:41
@git-nandor git-nandor force-pushed the INSTUI-4795_link_rework_v2 branch from c4d09d5 to 6c19e26 Compare December 4, 2025 10:11
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch 2 times, most recently from 5819ac6 to 9edded5 Compare December 4, 2025 10:32
@git-nandor git-nandor marked this pull request as draft December 4, 2025 10:35
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 9edded5 to e99d135 Compare December 4, 2025 10:50
@git-nandor git-nandor force-pushed the INSTUI-4795_link_rework_v2 branch from 6c19e26 to 49d787d Compare December 4, 2025 12:14
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from e99d135 to 4ac3730 Compare December 4, 2025 12:27
@git-nandor git-nandor force-pushed the INSTUI-4795_link_rework_v2 branch from 49d787d to acec849 Compare December 4, 2025 12:40
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch 2 times, most recently from 698f6c6 to 2ce570d Compare December 4, 2025 12:50
@git-nandor git-nandor force-pushed the INSTUI-4795_link_rework_v2 branch from acec849 to 5a59814 Compare December 4, 2025 15:08
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 2ce570d to 2cb89f5 Compare December 4, 2025 15:38
@git-nandor git-nandor marked this pull request as ready for review December 4, 2025 15:40
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 2cb89f5 to dd56bdb Compare December 4, 2025 15:52
Base automatically changed from INSTUI-4795_link_rework_v2 to v12 December 4, 2025 16:03
adamlobler
adamlobler requested changes Dec 4, 2025
View reviewed changes
Copy link
Collaborator

@adamlobler adamlobler left a comment

Choose a reason for hiding this comment

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

Other than the lastElement color issue I mentioned on Slack, I only found two minor things. We should update an icon color on the showcase, and we should check if we can improve the horizontal alignment of the elements inside the breadcrumb. They should be centered, but currently only the separators are centered, while the links are aligned to the top.
Image

@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from dd56bdb to 9992ee5 Compare December 5, 2025 07:56
@git-nandor git-nandor requested a review from adamlobler December 5, 2025 08:29
@git-nandor
Copy link
Contributor Author

git-nandor commented Dec 5, 2025

Other than the lastElement color issue...

The Breadcrumb or the Link doesn’t have a token that defines the color of non-interactive ‘plain’ text. It simply inherits the color from its environment. There is a color prop that can influence the color, but it has to be set manually—it doesn’t come from the theme automatically.

In this case, if we want the last non-interactive element of the Breadcrumb to also use the correct theme color, we need to wrap it with a component that automatically receives the proper colors from the theme, such as the Text component.

image

With this setup, the entire Breadcrumb will automatically switch to the correct colors based on the theme.

matyasf
matyasf requested changes Dec 5, 2025
View reviewed changes
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

Looks good, just a couple of small changes needed

@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 9992ee5 to 981fb4a Compare December 5, 2025 17:00
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 981fb4a to 4ac731b Compare December 5, 2025 17:03
@git-nandor
feat(ui-breadcrumb): migrate to new theming system and add new lucide…
a557fdf 
… icon and change separator style calculation
@git-nandor git-nandor force-pushed the INSTUI-4786_breadcrumb_rework_v3 branch from 4ac731b to a557fdf Compare December 5, 2025 17:37
git-nandor
git-nandor commented Dec 5, 2025
View reviewed changes
packages/ui-link/src/Link/styles.ts
verticalAlign: 'baseline',
fontSize: variantStyles.fontSize,
lineHeight: variantStyles.lineHeight,
color: componentTheme.unstyledTextColor,
Copy link
Contributor Author

@git-nandor git-nandor Dec 5, 2025

Choose a reason for hiding this comment

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

The unstyledTextColor token was added to the Link component to provide a theme-aware text color for non-interactive Link elements. This resolves the color issue not only in Link itself, but also for the last items in the Breadcrumb.

@git-nandor git-nandor requested a review from matyasf December 5, 2025 17:53
matyasf
matyasf approved these changes Dec 8, 2025
View reviewed changes
Copy link
Collaborator

@matyasf matyasf left a comment

Choose a reason for hiding this comment

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

looks good now, nice work!

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

Reviewers

@matyasf matyasf matyasf approved these changes

@HerrTopi HerrTopi HerrTopi approved these changes

@adamlobler adamlobler Awaiting requested review from adamlobler

Requested changes must be addressed to merge this pull request.

Assignees

@git-nandor git-nandor

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@git-nandor @matyasf @HerrTopi @adamlobler