Align icon-only buttons on the shared IconButton component #300

New Issue

opus-5 · 2026-08-01T05:59:36+02:00

opus-5 commented

2026-08-01 05:59:36 +02:00

The notification bell in the top bar sits visibly higher than the search
and theme buttons next to it, and renders larger. The cause is not the
glyph: the bell is the only one of the three that uses neither the shared
IconButton component nor its class.

Root cause

IconButton (apps/web/src/components/IconButton.tsx) renders a button
carrying the icon-button class, whose rule at
apps/web/src/styles/base.css:557 forces a 2rem box with
display: inline-flex; align-items: center; justify-content: center, and
normalises the icon to 1.15rem (base.css:571).

.notifications-bell__button (base.css:3814) has neither rule:

No flex context, so the <svg> is laid out as an inline element on the
text baseline of a 1.1rem line box. The space reserved below it for
descenders pushes it optically upwards — this is the misalignment.
No icon size rule, so the bell renders at lucide's default 24px instead
of the 18.4px the others use. It is bigger as well as higher.

It is also the only one of the three without a :focus-visible rule, and
there is no global one to fall back on (base.css defines
:focus-visible only per component: .icon-button, .toolbar-button,
.skip-link, .sidebar-resizer, .field input, .editor-page__title).
The bell therefore shows the browser default outline where every other
icon button shows the 2px accent ring.

Two kinds of outlier

Neither component nor class — the bell. Wrong alignment, wrong
size, inconsistent focus ring.
Class applied by hand to a raw <button> — e.g.
apps/web/src/layout/Sidebar.tsx:400,408,420,
apps/web/src/labels/LabelPicker.tsx:166,
apps/web/src/files/PondFileManager.tsx:74,
apps/web/src/layout/TopBar.tsx:113. These inherit the visuals but
bypass the component's contract, which is what guarantees an
aria-label and a title on an icon-only control.

Scope

Every icon-only button in the web app, not just the top bar.

Out of scope: the editor toolbar's .toolbar-button
(apps/web/src/editor/LinkMenu.tsx and friends) is a deliberately
separate visual system and stays as it is. Icons sitting inside buttons
that already carry a visible text label are not icon-only controls and
stay as they are too.

Files using lucide icons without IconButton today, to be audited one by
one — conversion only where the control is genuinely icon-only:

apps/web/src/access/AccessRulesManager.tsx
apps/web/src/files/PondFileManager.tsx
apps/web/src/import/ImportControl.tsx
apps/web/src/labels/LabelManager.tsx
apps/web/src/labels/LabelPicker.tsx
apps/web/src/layout/Sidebar.tsx
apps/web/src/members/MemberManager.tsx
apps/web/src/notifications/NotificationsBell.tsx
apps/web/src/pages/PageEditorPage.tsx
apps/web/src/pages/PluginManager.tsx
apps/web/src/watches/WatchesSection.tsx

Approach

Route every icon-only button through IconButton and drop the
hand-written duplicates of its CSS. For the bell this means
.notifications-bell__button disappears; the unread badge is positioned
absolutely against that button today, so it has to be re-fitted to the
2rem box (the wrapper div.notifications-bell keeps position: relative
for the dropdown).

IconButton currently accepts label, active and the native button
props. The bell additionally needs aria-haspopup and aria-expanded,
which pass through ...rest unchanged — no component change expected,
but verify rather than assume.

Guard against future drift

Add an ESLint rule that rejects className containing icon-button on a
raw JSX <button> element and points at the component instead. This
catches the next hand-glued outlier at authoring time. The rule must not
fire on IconButton's own implementation.

Accessibility

Covered by construction once the controls go through IconButton:
identical focus ring, and aria-label plus title enforced by the
component's required label prop. To be confirmed during the work:

Every converted control keeps a correct, localised accessible name — no
bare icon without a name.
Keyboard reachability and focus order are unchanged.
The bell keeps aria-haspopup="menu" and a correct aria-expanded.
The unread badge is not announced as a separate control; the unread
count belongs in the bell's accessible name or an appropriate live
region rather than as loose text.

Acceptance criteria

Search, bell and theme toggle are optically identical in size and
vertical alignment in the running app, at the default zoom and at
200%.
All three show the same focus ring on keyboard focus.
No raw <button className="icon-button …"> remains outside
IconButton itself; the ESLint rule fails the build if one is
added.
.notifications-bell__button is gone from base.css.
The unread badge sits correctly on the new box, with 0, 1, 9 and
99+ unread items.
pnpm lint, pnpm typecheck and the a11y spec pass.
Visual check by the operator before acceptance.

The notification bell in the top bar sits visibly higher than the search and theme buttons next to it, and renders larger. The cause is not the glyph: the bell is the only one of the three that uses neither the shared `IconButton` component nor its class. ## Root cause `IconButton` (`apps/web/src/components/IconButton.tsx`) renders a button carrying the `icon-button` class, whose rule at `apps/web/src/styles/base.css:557` forces a 2rem box with `display: inline-flex; align-items: center; justify-content: center`, and normalises the icon to `1.15rem` (`base.css:571`). `.notifications-bell__button` (`base.css:3814`) has neither rule: - No flex context, so the `<svg>` is laid out as an inline element on the text baseline of a `1.1rem` line box. The space reserved below it for descenders pushes it optically upwards — this is the misalignment. - No icon size rule, so the bell renders at lucide's default 24px instead of the 18.4px the others use. It is bigger as well as higher. It is also the only one of the three without a `:focus-visible` rule, and there is no global one to fall back on (`base.css` defines `:focus-visible` only per component: `.icon-button`, `.toolbar-button`, `.skip-link`, `.sidebar-resizer`, `.field input`, `.editor-page__title`). The bell therefore shows the browser default outline where every other icon button shows the 2px accent ring. ## Two kinds of outlier 1. **Neither component nor class** — the bell. Wrong alignment, wrong size, inconsistent focus ring. 2. **Class applied by hand to a raw `<button>`** — e.g. `apps/web/src/layout/Sidebar.tsx:400,408,420`, `apps/web/src/labels/LabelPicker.tsx:166`, `apps/web/src/files/PondFileManager.tsx:74`, `apps/web/src/layout/TopBar.tsx:113`. These inherit the visuals but bypass the component's contract, which is what guarantees an `aria-label` and a `title` on an icon-only control. ## Scope Every icon-only button in the web app, not just the top bar. Out of scope: the editor toolbar's `.toolbar-button` (`apps/web/src/editor/LinkMenu.tsx` and friends) is a deliberately separate visual system and stays as it is. Icons sitting inside buttons that already carry a visible text label are not icon-only controls and stay as they are too. Files using lucide icons without `IconButton` today, to be audited one by one — conversion only where the control is genuinely icon-only: - `apps/web/src/access/AccessRulesManager.tsx` - `apps/web/src/files/PondFileManager.tsx` - `apps/web/src/import/ImportControl.tsx` - `apps/web/src/labels/LabelManager.tsx` - `apps/web/src/labels/LabelPicker.tsx` - `apps/web/src/layout/Sidebar.tsx` - `apps/web/src/members/MemberManager.tsx` - `apps/web/src/notifications/NotificationsBell.tsx` - `apps/web/src/pages/PageEditorPage.tsx` - `apps/web/src/pages/PluginManager.tsx` - `apps/web/src/watches/WatchesSection.tsx` ## Approach Route every icon-only button through `IconButton` and drop the hand-written duplicates of its CSS. For the bell this means `.notifications-bell__button` disappears; the unread badge is positioned absolutely against that button today, so it has to be re-fitted to the 2rem box (the wrapper `div.notifications-bell` keeps `position: relative` for the dropdown). `IconButton` currently accepts `label`, `active` and the native button props. The bell additionally needs `aria-haspopup` and `aria-expanded`, which pass through `...rest` unchanged — no component change expected, but verify rather than assume. ## Guard against future drift Add an ESLint rule that rejects `className` containing `icon-button` on a raw JSX `<button>` element and points at the component instead. This catches the next hand-glued outlier at authoring time. The rule must not fire on `IconButton`'s own implementation. ## Accessibility Covered by construction once the controls go through `IconButton`: identical focus ring, and `aria-label` plus `title` enforced by the component's required `label` prop. To be confirmed during the work: - Every converted control keeps a correct, localised accessible name — no bare icon without a name. - Keyboard reachability and focus order are unchanged. - The bell keeps `aria-haspopup="menu"` and a correct `aria-expanded`. - The unread badge is not announced as a separate control; the unread count belongs in the bell's accessible name or an appropriate live region rather than as loose text. ## Acceptance criteria - [ ] Search, bell and theme toggle are optically identical in size and vertical alignment in the running app, at the default zoom and at 200%. - [ ] All three show the same focus ring on keyboard focus. - [ ] No raw `<button className="icon-button …">` remains outside `IconButton` itself; the ESLint rule fails the build if one is added. - [ ] `.notifications-bell__button` is gone from `base.css`. - [ ] The unread badge sits correctly on the new box, with 0, 1, 9 and 99+ unread items. - [ ] `pnpm lint`, `pnpm typecheck` and the a11y spec pass. - [ ] Visual check by the operator before acceptance.

opus-5 added this to the M33 — Tweaks & Feinschliff milestone 2026-08-01 05:59:36 +02:00

opus-5 added the

frontend

effort:S

labels 2026-08-01 05:59:36 +02:00

fable-5 referenced this issue from a commit

2026-08-01 06:56:16 +02:00

#300: route icon-only controls through IconButton/IconLink

opus-5 referenced a pull request that will close this issue

2026-08-01 06:56:53 +02:00

#300: route icon-only controls through IconButton/IconLink #308

opus-5 closed this issue