Use nldd-icon-button and nldd-activity-indicator for the card actions #236
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "style/nldd-icon-buttons"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Replaces five hand-rolled icon buttons and one hand-rolled spinner with the NLDD components, then fixes the three regressions that only showed up in the running app.
The swaps
CardRowquick actions + "Open in Mattermost", both variants (5 sites)<button>/<a>nldd-icon-button size="xs" variant="neutral-transparent"@-mention loading rowborder-top-color+@keyframesrotatenldd-activity-indicator size="16"The geometry is a dead-on match at
xs: 24x24 box, 16px icon, 4px radius, all three unchanged.The three regressions, and what they taught
The hover plate came back.
neutral-transparenthard-codes a transparent hover background, so the quick actions lost their grey chip whileBubbleMenuButtonbeside them kept one. The value is a literal in the shadow stylesheet, so no variant and no--semantics-buttons-*token reaches it. But NLDD sets no background on the host, and a transparent inner control lets the host's own background through - so this is public CSS on our own element, not the private--_is-hovered-background-color. Measured back tooklch(0.923 0.009 257.4)light andoklch(0.348 0.02 257.4)dark, identical to whatBubbleMenuButtonpaints.display: flexon the host fixes three things at once.:hostisinline-block, so it built a 27px line box around a 24px control. That overhang produced a strip of the card's pointer cursor under every button, a 1.5px misalignment againstBubbleMenuButton, and would have made the hover plate 3px too tall. The hosts measure 24x24 again.The cursor. Dragging along the row flipped arrow, hand, arrow, hand. Two causes: the card's own pointer leaking through the 2px gaps and the 3px overhang, and the
hrefvariant taking--semantics-controls-link-cursor, whichbase.css:99sets topointer. Waggle's convention (base.css:94) is that buttons keep the arrow and the pointer is for links; an icon button is a button whatever element it renders. Scoping the token on the group settles all four atdefaultand leaves every real text link alone. Measured: all fourdefault, all four 24x24.A skipped tab stop.
useCardKeyboardNav'sFOCUSABLE_SELECTORonly knew light-DOM elements, so none of the three hosts counted and the jump to the next section fired one control early - "Open in Mattermost" was unreachable by Tab.What has no test, and why
The tab-stop fix has no unit test.
getFocusableWithinfilters onoffsetParent, which happy-dom does not populate, so a custom element that is not already the active element is filtered out before any assertion can see it. Writing a test that passes anyway would be worse than none. Verified in Chromium on the built bundle instead, together with the cursor and the plate.Left out on purpose
The analysis found 19 hand-rolled icon buttons, not 10. These five are the ones where the box does not change, so only colour and hover move. The rest each have a reason to wait:
ReactionBar's add button is a bordered pill matching its badge siblings;MmSourceBreadcrumb's overflow trigger is inline text with no box;ImageLightboxsits on a black backdrop whereneutral-transparentinverts to near-black; the two preview modals have 36px neighbours that neither size step matches;BubbleMenuButtonserves two variants from one element.Verification
vue-tsc, eslint (0 errors), vitest 691 in 94 files,vite build, Playwright e2e 66 passed / 3 skipped,pre-commit run --all-files. Cursor, hover plate and box size measured in Chromium against the built bundle.1abf0badf8317b391985Closing: this is being handled elsewhere.
For whoever picks it up, what was in here and what it cost to find:
CardRow(quick actions plus the Mattermost link, both variants) tonldd-icon-button size="xs", and the @-mention loading spinner tonldd-activity-indicator.neutral-transparenthard-codes a transparent hover background, so the plate has to be painted on the host - public CSS, not the private--_is-hovered-background-color); the cursor flips along the row (the card's pointer leaks through the 2px gaps and through a 3px overhang from:host { display: inline-block }, fixed withdisplay: flexon the host); anduseCardKeyboardNav'sFOCUSABLE_SELECTORonly knew light-DOM elements, so Tab skipped "Open in Mattermost".BubbleMenuButtonhad to move to the same two--semantics-buttons-neutral-transparent-*tokens or it stayed visibly lighter than its swapped neighbours.Worth knowing: none of that is unit-testable here.
getFocusableWithinfilters onoffsetParent, which happy-dom does not populate, so a custom element that is not already the active element is filtered out before any assertion can see it. It was verified by measuring in Chromium on the built bundle.Opgegaan in #245, samen met #239 en #242. De commits zijn ongewijzigd overgenomen; #245 legt er de icoon-semantiek bovenop, die anders met #242 in conflict kwam.
Pull request closed