feat(2-breadcrumb): add trailing icon and disabled item support#668
feat(2-breadcrumb): add trailing icon and disabled item support#668paanSinghCoder wants to merge 7 commits intofeat/breadcrumb-auto-ellipsisfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
paanSinghCoder
changed the title
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
…le="presentation" and aria-hidden="true"
paanSinghCoder
changed the title
…and-disabled-item
| }, | ||
| ref | ||
| ) => { | ||
| const renderedElement = as ?? <a ref={ref} />; |
There was a problem hiding this comment.
We can handle rendering as span here itself. If it's current we can make it span here, instead of duplicating return logic
| if (disabled) { | ||
| return ( | ||
| <li className={cx(styles['breadcrumb-item'], className)}> | ||
| <span | ||
| ref={ref as React.RefObject<HTMLSpanElement>} | ||
| className={cx( | ||
| styles['breadcrumb-link'], | ||
| styles['breadcrumb-link-disabled'] | ||
| )} | ||
| aria-disabled='true' | ||
| > | ||
| {label} | ||
| </span> | ||
| </li> | ||
| ); | ||
| } | ||
| if (current) { | ||
| return ( | ||
| <li className={cx(styles['breadcrumb-item'], className)}> | ||
| <span | ||
| ref={ref as React.RefObject<HTMLSpanElement>} | ||
| className={cx( | ||
| styles['breadcrumb-link'], | ||
| styles['breadcrumb-link-active'] | ||
| )} | ||
| aria-current='page' | ||
| > | ||
| {label} | ||
| </span> | ||
| </li> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Duplicated logic just for two diff properties. Can be handled together. It would be better to just skip this and update the renderedElement to render as span, and update classname and properties below
2 participants
Breadcrumb 2nd PR
Issue-600
Description
role="presentation"andaria-hidden="true"toSaperator.<a>.Type of Change
How Has This Been Tested?
Manual and tests
Checklist:
Screenshots (if appropriate):
[Add screenshots here]
Related Issues
[Link any related issues here using #issue-number]