We adopted <Breadcrumb> across 25 view files in two apps - it is the one component in the library that was an unambiguous upgrade on our hand-rolled markup (we had a flat <nav> of spans with no <ol>/<li>, no aria-current, and separators that read aloud as content). Both bugs below are in the props we did not use, and we found them auditing before widening adoption.
Both reproduced by rendering the component on main (a198835, 0.2.343) through renderTemplateString, counting <li> in the output.
1. maxItems always renders one item too many, and maxItems: 1 renders more than no collapse at all
Breadcrumb.stx:9-12:
if (maxItems > 0 && items.length > maxItems) {
const firstItems = items.slice(0, 1)
const lastItems = items.slice(-(maxItems - 1))
displayItems = [...firstItems, { label: '...', href: '#', collapsed: true }, ...lastItems]
}The result is 1 + 1 + (maxItems - 1) = maxItems + 1 items. Rendered <li> counts for a five-item trail a b c d e:
maxItems | rendered <li> | labels |
|---|---|---|
| 0 (off) | 5 | a b c d e |
| 1 | 7 | a … a b c d e |
| 2 | 3 | a … e |
| 3 | 4 | a … d e |
| 4 | 5 | a … c d e |
maxItems: 2 is the only value that keeps its promise, and it does so by coincidence.
maxItems: 1 is the sharp edge: -(1 - 1) is -0, and slice(-0) === slice(0), so lastItems is the entire array. Asking for one crumb renders seven, with a duplicated - the first crumb appears both before and after the ellipsis.
Suggested fix - reserve one slot for the ellipsis and clamp:
if (maxItems > 0 && items.length > maxItems) {
const tail = Math.max(0, maxItems - 2) // first + ellipsis + tail === maxItems
const lastItems = tail > 0 ? items.slice(-tail) : []
displayItems = [items[0], { label: '...', href: '#', collapsed: true }, ...lastItems]
}That gives maxItems: 3 -> a … e, and maxItems: 1 -> a …, which is the most honest thing a one-slot breadcrumb can show.
2. The icon prop never renders - it is written into an attribute called innerHTML
Breadcrumb.stx:41 and :56:
<span innerHTML=""></span>innerHTML is not an HTML attribute, and stx has no directive by that name, so the markup lands in the attribute value and the span stays empty. Rendered output for items: [{ label: 'Home', href: '/', icon: '<svg id=MYICON></svg>' }, …]:
<span class="inline-flex items-center gap-1.5">
<span innerHTML="<svg id=MYICON></svg>"></span> <!-- empty in the browser -->
<span>Home</span>
</span>The icon is silently dropped - no error, no warning, and the surrounding inline-flex … gap-1.5 wrapper still reserves the gap, so the label sits with a stray space in front of it.
The library's own convention is x-html, which Tabs.stx:158 and :180 both use for exactly this - injecting a caller-supplied icon string. Breadcrumb.stx is the only file in all 102 components that uses an innerHTML= attribute:
$ grep -rn 'innerHTML=' packages/components/src/ui/
packages/components/src/ui/breadcrumb/Breadcrumb.stx:41
packages/components/src/ui/breadcrumb/Breadcrumb.stx:56so swapping both to x-html="…" matches the rest of the library.
One thing worth deciding alongside the fix: and x-html both inject unescaped, and item.icon comes straight from the caller's array. That is fine for a literal icon string, but a breadcrumb built from record names is a natural place for untrusted text to arrive, so it is worth documenting that icon is trusted HTML - or accepting an icon class instead, which is what SidebarPinned.stx:78 does (class=" h-[20px] w-[20px]") and which needs no raw injection at all.
Happy to send a PR for both.
Environment: main @ a198835, @stacksjs/components 0.2.343.