Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughReworks Breadcrumb API, demos, docs, styles, and tests: adds trailingIcon and disabled support, changes dropdownItems to pass Menu.Item props (including link rendering), renders current/disabled items as non-anchors, updates separator accessibility/ref handling, and exposes a shopping-bag icon in the demo scope. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BreadcrumbItem
participant Menu
participant DOM
User->>BreadcrumbItem: click
alt item has dropdown && not disabled
BreadcrumbItem->>Menu: open menu (render Menu.Items from dropdownItems)
Menu->>DOM: render list of Menu.Item (may use <a> via render/href)
User->>Menu: click Menu.Item -> triggers item action
else item is disabled or current
BreadcrumbItem->>DOM: render <span> (aria-disabled/aria-current, no anchor)
User->>BreadcrumbItem: click -> no menu / no navigation
end
BreadcrumbItem->>DOM: render Separator (role="presentation", aria-hidden="true")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Important Merge conflicts detected (Beta)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/raystack/components/breadcrumb/breadcrumb-root.tsx (3)
148-166:keyedhelper redundantly checksisValidElement.The
keyedfunction is only ever called with elements frombeforeItemsandafterItems, which are already validated asReactElementinstances fromparseBreadcrumbChildren. TheisValidElement(el)check is unnecessary.♻️ Optional simplification
- const keyed = (el: ReactElement, k: string) => - isValidElement(el) && el.key != null - ? el - : cloneElement(el, { key: k }); + const keyed = (el: ReactElement, k: string) => + el.key != null ? el : cloneElement(el, { key: k });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/breadcrumb/breadcrumb-root.tsx` around lines 148 - 166, The keyed helper unnecessarily calls isValidElement; since beforeItems/afterItems are already ReactElement instances from parseBreadcrumbChildren, remove the isValidElement check in the keyed function (breadcrumb-root.tsx) and simply return the element if el.key != null, otherwise return cloneElement(el, { key: k }); Update the keyed helper used where beforeItems/afterItems are iterated so it only checks el.key and clones when missing, keeping cloneElement usage and keys (e.g., calls in the beforeItems.forEach and afterItems.forEach loops) intact.
69-88: Consider handling unrecognized children gracefully.The
parseBreadcrumbChildrenfunction silently drops children that don't match known breadcrumb parts (Item, Separator, Ellipsis). This could lead to confusion if users pass custom elements or text nodes. Consider either logging a warning in development or documenting this behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/breadcrumb/breadcrumb-root.tsx` around lines 69 - 88, parseBreadcrumbChildren currently drops children that aren't recognized breadcrumb parts (BreadcrumbItem, BreadcrumbSeparator, BreadcrumbEllipsis) with no feedback; update parseBreadcrumbChildren to warn in development when an element is a valid React element but not matched by isBreadcrumbPart (and optionally when a non-empty text node or unknown node is passed) by calling console.warn or process.env.NODE_ENV check inside the flat.forEach loop; reference the existing symbols parseBreadcrumbChildren, flattenFragments(Children.toArray(children)), isValidElement, isBreadcrumbPart, BreadcrumbItem, BreadcrumbSeparator, and BreadcrumbEllipsis and emit a clear dev-only warning that includes the offending child/type so users know why their node was dropped.
171-173: Unused_propsChildrenextraction appears defensive but unnecessary.The destructuring on line 171-173 extracts
childrenfrompropsto avoid passing it twice to thenav. However,childrenwas already destructured at line 116, so it shouldn't be inprops. This suggests the typing could be tightened, but it's harmless as written.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/raystack/components/breadcrumb/breadcrumb-root.tsx` around lines 171 - 173, The extraction of children into _propsChildren from props (const { children: _propsChildren, ...restProps } = props) is redundant because children is already destructured earlier; remove that destructuring and ensure you use the existing children variable and a restProps that does not re-extract children (or tighten the prop typing to exclude children from props) so children is not handled twice in BreadcrumbRoot; update references to use restProps (or a propsWithoutChildren alias) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/www/src/app/examples/page.tsx`:
- Around line 61-66: The Breadcrumb.Item for the "Footwear" label has the wrong
href; update the href on the Breadcrumb.Item (the JSX element with label
"Footwear") to the correct path (e.g. change
'/products/electronics/laptops/gaming/accessories' to
'/products/electronics/laptops/gaming/accessories/footwear') so the link matches
the label.
---
Nitpick comments:
In `@packages/raystack/components/breadcrumb/breadcrumb-root.tsx`:
- Around line 148-166: The keyed helper unnecessarily calls isValidElement;
since beforeItems/afterItems are already ReactElement instances from
parseBreadcrumbChildren, remove the isValidElement check in the keyed function
(breadcrumb-root.tsx) and simply return the element if el.key != null, otherwise
return cloneElement(el, { key: k }); Update the keyed helper used where
beforeItems/afterItems are iterated so it only checks el.key and clones when
missing, keeping cloneElement usage and keys (e.g., calls in the
beforeItems.forEach and afterItems.forEach loops) intact.
- Around line 69-88: parseBreadcrumbChildren currently drops children that
aren't recognized breadcrumb parts (BreadcrumbItem, BreadcrumbSeparator,
BreadcrumbEllipsis) with no feedback; update parseBreadcrumbChildren to warn in
development when an element is a valid React element but not matched by
isBreadcrumbPart (and optionally when a non-empty text node or unknown node is
passed) by calling console.warn or process.env.NODE_ENV check inside the
flat.forEach loop; reference the existing symbols parseBreadcrumbChildren,
flattenFragments(Children.toArray(children)), isValidElement, isBreadcrumbPart,
BreadcrumbItem, BreadcrumbSeparator, and BreadcrumbEllipsis and emit a clear
dev-only warning that includes the offending child/type so users know why their
node was dropped.
- Around line 171-173: The extraction of children into _propsChildren from props
(const { children: _propsChildren, ...restProps } = props) is redundant because
children is already destructured earlier; remove that destructuring and ensure
you use the existing children variable and a restProps that does not re-extract
children (or tighten the prop typing to exclude children from props) so children
is not handled twice in BreadcrumbRoot; update references to use restProps (or a
propsWithoutChildren alias) accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65b1ecac-7a92-4967-8461-a9e16b6675b4
📒 Files selected for processing (7)
apps/www/src/app/examples/page.tsxapps/www/src/components/demo/demo.tsxapps/www/src/content/docs/components/breadcrumb/demo.tsapps/www/src/content/docs/components/breadcrumb/index.mdxapps/www/src/content/docs/components/breadcrumb/props.tspackages/raystack/components/breadcrumb/__tests__/breadcrumb.test.tsxpackages/raystack/components/breadcrumb/breadcrumb-root.tsx
apps/www/src/app/examples/page.tsx
Outdated
Comment on lines
+61
to
+66
| <Breadcrumb.Item | ||
| href='/products/electronics/laptops/gaming/accessories' | ||
| current | ||
| > | ||
| Footwear | ||
| </Breadcrumb.Item> |
Contributor
There was a problem hiding this comment.
Inconsistent href for "Footwear" item.
The href for "Footwear" is /products/electronics/laptops/gaming/accessories but the label is "Footwear". This appears to be a copy-paste error—the href should likely be /products/electronics/laptops/gaming/accessories/footwear or similar.
🐛 Suggested fix
<Breadcrumb.Item
- href='/products/electronics/laptops/gaming/accessories'
+ href='/products/electronics/laptops/gaming/accessories/footwear'
current
>
Footwear
</Breadcrumb.Item>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <Breadcrumb.Item | |
| href='/products/electronics/laptops/gaming/accessories' | |
| current | |
| > | |
| Footwear | |
| </Breadcrumb.Item> | |
| <Breadcrumb.Item | |
| href='/products/electronics/laptops/gaming/accessories/footwear' | |
| current | |
| > | |
| Footwear | |
| </Breadcrumb.Item> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/www/src/app/examples/page.tsx` around lines 61 - 66, The Breadcrumb.Item
for the "Footwear" label has the wrong href; update the href on the
Breadcrumb.Item (the JSX element with label "Footwear") to the correct path
(e.g. change '/products/electronics/laptops/gaming/accessories' to
'/products/electronics/laptops/gaming/accessories/footwear') so the link matches
the label.
paanSinghCoder
changed the title
paanSinghCoder
changed the title
paanSinghCoder
changed the title
rohanchkrabrty
requested changes
Mar 13, 2026
* feat: add disabled prop support for item * feat: add trailing item support from item * feat(breadcrumb): update separator accessibility attributes to use role="presentation" and aria-hidden="true" * fix: replace <a> with <span> for current item * style(breadcrumb): enhance active link styling with hover effect and default cursor * chore: Remove breadcrumb examples * refactor: merge jsx * feat (3-breadcrumb): add href support and expose data-disabled and data-current attributes (#669) * feat (breadcrumb-dropdown): add href support * feat: add data attributes * refactor: merge react 19 * refactor: pass all menuItem props
rohanchkrabrty
approved these changes
Mar 21, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx`:
- Around line 92-109: The current branch returns a <span> for disabled/current
items but drops shared DOM props because ...(props) isn’t forwarded; update the
BreadcrumbItem component to forward remaining common DOM attributes (data-*,
aria-*, id, title, role, etc.) to the rendered <span
className={cx(styles['breadcrumb-link'], ...)} ref={ref} while filtering out
anchor-only props like href, target, rel; ensure the existing conditional
additions ('aria-disabled', 'data-disabled', 'aria-current', 'data-current') are
merged with the forwarded props so tests and accessibility attributes are
preserved when disabled or current.
- Around line 59-89: The dropdown branch in Breadcrumb.Item returns a Menu
directly which breaks list semantics and places the component-level className
and ref on Menu.Trigger; wrap the entire Menu in the same <li> container used by
the other branch so the item container receives className, ref and any list
semantics. Specifically, move/attach the className (styles['breadcrumb-item'] +
className) and ref to the <li> element that wraps the Menu, keep Menu.Trigger
receiving only button-specific props (exclude the item container className), and
preserve passing dropdownItems into Menu.Content/Item as currently implemented
in breadcrumb-item.tsx (identify Menu, Menu.Trigger, Menu.Content, Menu.Item and
the dropdownItems mapping).
- Around line 115-123: The cloneElement call in breadcrumb-item.tsx currently
sets className: styles['breadcrumb-link'] but then spreads
...renderedElement.props which can overwrite that className; update the
cloneElement props to merge classes using cx so the base breadcrumb-link is
preserved and any custom className from renderedElement.props or incoming props
is combined (e.g. className: cx(styles['breadcrumb-link'], props.className,
renderedElement.props?.className)); update the reference in the cloneElement
call (in the component that uses renderedElement/ref) and add a unit/test that
renders the component with as={<a className="custom" />} asserting both
styles['breadcrumb-link'] and "custom" are present to prevent regressions.
In `@packages/raystack/components/breadcrumb/breadcrumb-misc.tsx`:
- Around line 33-39: The separator currently spreads {...props} after setting
role='presentation' and aria-hidden='true' which allows callers to override
those semantics; update the Breadcrumb separator in breadcrumb-misc.tsx (the
element using ref, className, cx(styles['breadcrumb-separator'], className), and
{...props}) so the user props are applied before the fixed accessibility
attributes or remove role/aria from the public props by destructuring them out
(e.g., const { role, 'aria-hidden': ariaHidden, ...rest } = props) and then
spread rest, finally setting role='presentation' and aria-hidden='true' on the
element to ensure the separator semantics are non-overridable.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4fb8ed04-2ea0-41b3-93db-03c29a3a4455
📒 Files selected for processing (8)
apps/www/src/components/playground/breadcrumb-examples.tsxapps/www/src/content/docs/components/breadcrumb/demo.tsapps/www/src/content/docs/components/breadcrumb/index.mdxapps/www/src/content/docs/components/breadcrumb/props.tspackages/raystack/components/breadcrumb/__tests__/breadcrumb.test.tsxpackages/raystack/components/breadcrumb/breadcrumb-item.tsxpackages/raystack/components/breadcrumb/breadcrumb-misc.tsxpackages/raystack/components/breadcrumb/breadcrumb.module.css
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/www/src/content/docs/components/breadcrumb/index.mdx
- apps/www/src/content/docs/components/breadcrumb/props.ts
- apps/www/src/content/docs/components/breadcrumb/demo.ts
Comment on lines
+59
to
89
| if (dropdownItems && !disabled) { | ||
| return ( | ||
| <Menu> | ||
| <Menu.Trigger className={styles['breadcrumb-dropdown-trigger']}> | ||
| <Menu.Trigger | ||
| ref={ref as React.Ref<HTMLButtonElement>} | ||
| className={cx(styles['breadcrumb-dropdown-trigger'], className)} | ||
| {...(props as React.ButtonHTMLAttributes<HTMLButtonElement>)} | ||
| > | ||
| {label} | ||
| <ChevronDownIcon className={styles['breadcrumb-dropdown-icon']} /> | ||
| </Menu.Trigger> | ||
| <Menu.Content className={styles['breadcrumb-dropdown-content']}> | ||
| {dropdownItems.map((dropdownItem, dropdownIndex) => ( | ||
| <Menu.Item | ||
| key={dropdownItem.key ?? dropdownIndex} | ||
| className={styles['breadcrumb-dropdown-item']} | ||
| onClick={dropdownItem?.onClick} | ||
| > | ||
| {dropdownItem.label} | ||
| </Menu.Item> | ||
| ))} | ||
| {dropdownItems.map((dropdownItem, dropdownIndex) => { | ||
| const { | ||
| key, | ||
| className: itemClassName, | ||
| ...menuItemProps | ||
| } = dropdownItem; | ||
| return ( | ||
| <Menu.Item | ||
| key={key ?? dropdownIndex} | ||
| className={cx( | ||
| styles['breadcrumb-dropdown-item'], | ||
| itemClassName | ||
| )} | ||
| {...menuItemProps} | ||
| /> | ||
| ); | ||
| })} | ||
| </Menu.Content> | ||
| </Menu> |
Contributor
There was a problem hiding this comment.
Wrap the dropdown variant in the same <li> container.
This is now the only Breadcrumb.Item branch that returns something other than a list item. Inside the breadcrumb’s ordered list, that breaks list semantics and also makes className land on the trigger instead of the item container.
Suggested fix
if (dropdownItems && !disabled) {
return (
- <Menu>
- <Menu.Trigger
- ref={ref as React.Ref<HTMLButtonElement>}
- className={cx(styles['breadcrumb-dropdown-trigger'], className)}
- {...(props as React.ButtonHTMLAttributes<HTMLButtonElement>)}
- >
- {label}
- <ChevronDownIcon className={styles['breadcrumb-dropdown-icon']} />
- </Menu.Trigger>
- <Menu.Content className={styles['breadcrumb-dropdown-content']}>
- {dropdownItems.map((dropdownItem, dropdownIndex) => {
- const {
- key,
- className: itemClassName,
- ...menuItemProps
- } = dropdownItem;
- return (
- <Menu.Item
- key={key ?? dropdownIndex}
- className={cx(
- styles['breadcrumb-dropdown-item'],
- itemClassName
- )}
- {...menuItemProps}
- />
- );
- })}
- </Menu.Content>
- </Menu>
+ <li className={cx(styles['breadcrumb-item'], className)}>
+ <Menu>
+ <Menu.Trigger
+ ref={ref as React.Ref<HTMLButtonElement>}
+ className={styles['breadcrumb-dropdown-trigger']}
+ {...(props as React.ButtonHTMLAttributes<HTMLButtonElement>)}
+ >
+ {label}
+ <ChevronDownIcon className={styles['breadcrumb-dropdown-icon']} />
+ </Menu.Trigger>
+ <Menu.Content className={styles['breadcrumb-dropdown-content']}>
+ {dropdownItems.map((dropdownItem, dropdownIndex) => {
+ const {
+ key,
+ className: itemClassName,
+ ...menuItemProps
+ } = dropdownItem;
+ return (
+ <Menu.Item
+ key={key ?? dropdownIndex}
+ className={cx(
+ styles['breadcrumb-dropdown-item'],
+ itemClassName
+ )}
+ {...menuItemProps}
+ />
+ );
+ })}
+ </Menu.Content>
+ </Menu>
+ </li>
);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 59
- 89, The dropdown branch in Breadcrumb.Item returns a Menu directly which
breaks list semantics and places the component-level className and ref on
Menu.Trigger; wrap the entire Menu in the same <li> container used by the other
branch so the item container receives className, ref and any list semantics.
Specifically, move/attach the className (styles['breadcrumb-item'] + className)
and ref to the <li> element that wraps the Menu, keep Menu.Trigger receiving
only button-specific props (exclude the item container className), and preserve
passing dropdownItems into Menu.Content/Item as currently implemented in
breadcrumb-item.tsx (identify Menu, Menu.Trigger, Menu.Content, Menu.Item and
the dropdownItems mapping).
Comment on lines
+92
to
+109
| if (disabled || current) { | ||
| return ( | ||
| <li className={cx(styles['breadcrumb-item'], className)}> | ||
| <span | ||
| ref={ref as React.RefObject<HTMLSpanElement>} | ||
| className={cx( | ||
| styles['breadcrumb-link'], | ||
| disabled && styles['breadcrumb-link-disabled'], | ||
| current && styles['breadcrumb-link-active'] | ||
| )} | ||
| {...(disabled && { | ||
| 'aria-disabled': 'true', | ||
| 'data-disabled': 'true' | ||
| })} | ||
| {...(current && { 'aria-current': 'page', 'data-current': 'true' })} | ||
| > | ||
| {label} | ||
| </span> |
Contributor
There was a problem hiding this comment.
Forward shared DOM props on current/disabled items.
This branch drops data-*, aria-*, id, title, and similar shared attributes because ...props never reaches the rendered <span>. That regresses test hooks and accessible names for exactly the two new non-anchor states introduced here. Please keep the shared DOM props on the span while filtering anchor-only ones such as href.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 92
- 109, The current branch returns a <span> for disabled/current items but drops
shared DOM props because ...(props) isn’t forwarded; update the BreadcrumbItem
component to forward remaining common DOM attributes (data-*, aria-*, id, title,
role, etc.) to the rendered <span className={cx(styles['breadcrumb-link'], ...)}
ref={ref} while filtering out anchor-only props like href, target, rel; ensure
the existing conditional additions ('aria-disabled', 'data-disabled',
'aria-current', 'data-current') are merged with the forwarded props so tests and
accessibility attributes are preserved when disabled or current.
Comment on lines
115
to
123
| {cloneElement( | ||
| renderedElement, | ||
| { | ||
| className: cx( | ||
| styles['breadcrumb-link'], | ||
| current && styles['breadcrumb-link-active'] | ||
| ), | ||
| className: styles['breadcrumb-link'], | ||
| href, | ||
| ...props, | ||
| ...renderedElement.props | ||
| ...renderedElement.props, | ||
| ref | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/raystack/components/breadcrumb/breadcrumb-item.tsx | head -130Repository: raystack/apsara
Length of output: 4479
🏁 Script executed:
find . -type f -name "*.test.*" -o -name "*.spec.*" | grep -i breadcrumb | head -5Repository: raystack/apsara
Length of output: 130
🏁 Script executed:
cat -n ./packages/raystack/components/breadcrumb/__tests__/breadcrumb.test.tsxRepository: raystack/apsara
Length of output: 22356
🏁 Script executed:
rg "cx\(" packages/raystack/components/breadcrumb/breadcrumb-item.tsx -A 2Repository: raystack/apsara
Length of output: 707
Merge the custom element class with breadcrumb-link.
When ...renderedElement.props spreads after className: styles['breadcrumb-link'], any className on the custom as element will override the component's base styling. Use cx() to merge them explicitly, and add a test for as={<a className='custom' />} to prevent regression.
Suggested fix
{cloneElement(
renderedElement,
{
- className: styles['breadcrumb-link'],
href,
...props,
...renderedElement.props,
+ className: cx(
+ styles['breadcrumb-link'],
+ renderedElement.props.className
+ ),
ref
},
label
)}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/breadcrumb/breadcrumb-item.tsx` around lines 115
- 123, The cloneElement call in breadcrumb-item.tsx currently sets className:
styles['breadcrumb-link'] but then spreads ...renderedElement.props which can
overwrite that className; update the cloneElement props to merge classes using
cx so the base breadcrumb-link is preserved and any custom className from
renderedElement.props or incoming props is combined (e.g. className:
cx(styles['breadcrumb-link'], props.className,
renderedElement.props?.className)); update the reference in the cloneElement
call (in the component that uses renderedElement/ref) and add a unit/test that
renders the component with as={<a className="custom" />} asserting both
styles['breadcrumb-link'] and "custom" are present to prevent regressions.
Comment on lines
+33
to
+39
| <span | ||
| ref={ref} | ||
| className={cx(styles['breadcrumb-separator'], className)} | ||
| role='presentation' | ||
| aria-hidden='true' | ||
| {...props} | ||
| > |
Contributor
There was a problem hiding this comment.
Keep the separator semantics non-overridable.
...props is spread after role='presentation' and aria-hidden='true', so a caller can accidentally re-expose the separator to assistive tech. Move the spread before the fixed attrs, or omit those two props from the public surface.
Suggested fix
<span
ref={ref}
+ {...props}
className={cx(styles['breadcrumb-separator'], className)}
role='presentation'
aria-hidden='true'
- {...props}
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <span | |
| ref={ref} | |
| className={cx(styles['breadcrumb-separator'], className)} | |
| role='presentation' | |
| aria-hidden='true' | |
| {...props} | |
| > | |
| <span | |
| ref={ref} | |
| {...props} | |
| className={cx(styles['breadcrumb-separator'], className)} | |
| role='presentation' | |
| aria-hidden='true' | |
| > |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/raystack/components/breadcrumb/breadcrumb-misc.tsx` around lines 33
- 39, The separator currently spreads {...props} after setting
role='presentation' and aria-hidden='true' which allows callers to override
those semantics; update the Breadcrumb separator in breadcrumb-misc.tsx (the
element using ref, className, cx(styles['breadcrumb-separator'], className), and
{...props}) so the user props are applied before the fixed accessibility
attributes or remove role/aria from the public props by destructuring them out
(e.g., const { role, 'aria-hidden': ariaHidden, ...rest } = props) and then
spread rest, finally setting role='presentation' and aria-hidden='true' on the
element to ensure the separator semantics are non-overridable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Breadcrumb 1st PR
Issue-600
Description
New props (root) - Reverted
-maxItems– Maximum number of breadcrumb items to show. When there are more items, the trail collapses: a fixed number at the start, ellipsis, then the rest at the end. At least 2 items are always visible (1 before and 1 after the ellipsis). Values < 2 are treated as 2.-itemsBeforeCollapse– Number of items to show before the ellipsis when collapsed. If not set, it is derived frommaxItems(e.g.maxItems={5}→ 2 before, 3 after). The count after the ellipsis is always derived.Docs updates
className, note oncurrentprop, size example with tabs).Type of Change
How Has This Been Tested?
[Describe the tests that you ran to verify your changes]
Checklist:
Screenshots (if appropriate):
[Add screenshots here]
Related Issues
[Link any related issues here using #issue-number]
Summary by CodeRabbit
New Features
Documentation
Accessibility
Style
Tests