Skip to content

AIDA-2338: Keep sidebar headings readable at large sizes - #2263

Merged
ravarora2 merged 2 commits into
mainfrom
rarora/AIDA-2338/sidebar-heading-reflow
Nov 3, 2026
Merged

ravarora2 merged 2 commits into
mainfrom
rarora/AIDA-2338/sidebar-heading-reflow

Conversation

@ravarora2

@ravarora2 ravarora2 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Why

At 200% interface size, a 220px sidebar can leave no width for “Channels” or “Direct messages”. The header reserves scaled button space inside a fixed-width sidebar. Fixes AIDA-2338.

What

Cap the space reserved for actions so headings retain useful text width. Keep the existing single-row height, adjacent chevron, full collapse target, and fading for long names.

How

Normal widths retain the original action clearance. At cramped enlarged sizes, the padding leaves room for the label and chevron. Hovered or keyboard-focused actions cover the trailing text with the panel surface, following the existing Inbox overlay behavior. Shared buttons, menus, and saved sidebar widths keep their existing owners.

Add one browser case for real text geometry, compact height, chevron placement, pointer targets, and keyboard focus. Extend the existing overflow-and-recovery case to cover a long custom name at 200%. No existing browser cases are removed. These contracts need a browser layout engine. The geometry matrix runs once; light and dark screenshots cover paint separately.

Risk

At the narrowest enlarged sizes, revealed actions cover the end of a heading. Long names still fade, and the full accessible names remain available. Default-size overflow and action alignment retain their existing contracts.

Testing

Local Chromium and WebKit fixtures exercised 100%, 120%, 150%, 180%, and 200% interface sizing at 220/260/520px sidebar widths. The complete heading, navigation, group-icon, and section-creation files passed 40 checks, including the previously failing overflow and narrow drag cases. Screenshots show the actual app UI with fixture messages at 100% and 200%, including both themes and revealed actions.

Mutation checks restore the original CSS at 200%/220px and the reviewed wrapping layout: the new regression case rejects missing label space and extra header rows respectively. The checks pass with the revised CSS.

All hosted browser journey shards passed at 3cbc353e1. CI required is currently blocked by the native media-stream test failure at media_stream_tests.rs:300, matching the known spool-progress race tracked by #1970. Human desktop acceptance remains pending. To try it: set Settings → Appearance → Interface size to 200%, narrow the sidebar fully, then hover and tab through section actions. Headers should remain compact; actions should open without collapsing their section.

Generated with Codex

Co-authored-by: BuzzPRReviewAssistant1 <a581a7e8115222acdc32ffb14bfce55d48c24478d98e155f8eba126b69b83c7b@buzz.block.builderlab.xyz>
Signed-off-by: Ravneet Arora <rarora@squareup.com>
@ravarora2
ravarora2 marked this pull request as ready for review November 3, 2026 16:09
@ravarora2
ravarora2 requested a review from a team as a code owner November 3, 2026 16:09

@kalvinnchau kalvinnchau left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Three layout/behavior regressions remain at this commit. The changed browser files pass, but existing overflow and section-creation cases fail; reverting only the CSS to the comparison base makes both pass.

.sectionHeader > details {
flex: 1 1 9rem;
min-width: 0;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [P2] Size the disclosure from its content before wrapping actions

At 100% interface size and a 220px sidebar, the fixed 9rem basis pushes the actions onto a second row even for “Channels”, doubling the header from 32px to 64px. Because the actions remain transparent until hover/focus, this appears as empty space. At 200%, measured Channels/DM headers grow from 64px to 168px/208px. Let actual title width determine wrapping (or preserve appropriately scaled reserved action space). Add a regression assertion that short titles and actions share a row when they fit; the new separation assertion currently accepts the extra row. Fix suggestions have not been tested.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Fixed in 3cbc353. Removed the fixed 9rem basis and wrapping header layout, restoring the original single-row height. The action reservation is now capped by available width using shared sizing tokens. The new geometry case checks compact height, action alignment, and the full disclosure target across five interface sizes and three sidebar widths. Restoring the reviewed stylesheet makes that case fail on compact height; the revision passes in Chromium and WebKit.

}
.sectionHeader > details .sectionTitle {
white-space: normal;
overflow-wrap: anywhere;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [P2] Reconcile unconditional wrapping with the existing overflow contract

These rules change long section names from fading to wrapping even at 100%. The existing navigation-group-icons.spec.mjs:213 case fails in Chromium and WebKit CI: wrapped=true, fits=true, clearOfActions=false, versus expected false/false/true. A local Chromium run fails at this head and passes with only this stylesheet restored to the base. Preserve the default-size overflow behavior or explicitly update the intended product contract and its regression coverage; leaving the conflicting test failing is not sufficient.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Fixed in 3cbc353. Restored the existing nowrap/FadingLabel behavior and default-size action clearance. The original overflow, clearOfActions, centering, and widened-recovery assertions in navigation-group-icons.spec.mjs are preserved and pass in both engines. Extended that same case to check useful text width and single-line fading for a long custom heading at 200%. No existing overflow assertion was weakened.

}
/* Keep disclosure actions in flow so enlarged controls cannot consume the
label's entire width. The rem basis follows the interface-size preference. */
.sectionHeader:has(> details) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 [P2] Preserve the narrow-sidebar create-section drag workflow

At 200% in a narrow sidebar, sidebar-section-creation.spec.mjs:382 fails to reveal [data-create-section-drop] during revealCreate (:228). This fails in Chromium CI and locally at the candidate, and passes locally when only Channels.module.css is reverted to the comparison base. The CSS dependency is reproduced; the precise collision mechanism remains unverified. Recheck and repair this drag workflow alongside the header geometry change before merging.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Fixed the reproduction in 3cbc353 by restoring the original compact header geometry. The complete sidebar-section-creation.spec.mjs file passes in Chromium and WebKit, including the 200% narrow-sidebar case and its existing ResizeObserver gate. No drag collision code, observer gate, or assertion was changed. The latest hosted browser journey shards also pass. I have not claimed a more specific collision-mechanism diagnosis.

@loganj loganj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Larry: an independent adversarial review of 23d83b9. Source review only; I did not run the app at this head.

Verdict: comment, not approve. I agree with the three findings already posted on this head, and CI confirms two of them: navigation-group-icons.spec.mjs:213 (fade on overflow) fails in Chromium and WebKit, and sidebar-section-creation.spec.mjs:382 (narrow create-section drag) fails in Chromium. Fixing them probably changes the layout approach, so the result needs another look.

Must fix

  • The two failing existing browser contracts above (error).

Additional findings (inline)

  1. The chevron moves to the far end of the disclosure at every size, 100% included (error, unless intended; then state it in the PR body).
  2. The summary's own flex-wrap lets the chevron drop to a row by itself, so a heading can take three rows (error).
  3. An alternative that keeps one row and the fade contract: overlay the hidden actions as Inbox rows already do (advice, untested).
  4. The light/dark loop doubles the geometry matrix without covering a layout difference (advice).

Question: before this change the summary spanned the whole header, with padding under the actions, so a click in the empty action area toggled the section. Now that area is outside the summary, which is why navigation-sidebar.spec.mjs moved from the summary edge to the header edge. Is the smaller toggle target intended?

Comment thread tests/browser/sidebar-headings.spec.mjs Resolved
Co-authored-by: BuzzPRReviewAssistant1 <a581a7e8115222acdc32ffb14bfce55d48c24478d98e155f8eba126b69b83c7b@buzz.block.builderlab.xyz>
Signed-off-by: Ravneet Arora <rarora@squareup.com>
@ravarora2

ravarora2 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Response to the review: pushed the fixes in 3cbc353 and replied to all three inline findings. Restored compact single-row headers and the existing fading/action-clearance contract; the narrow create-section drag case passes without changing its assertions or collision code. The four complete affected browser files passed 40 checks locally across Chromium and WebKit, and the latest hosted browser journey shards pass. The remaining native media-stream CI failure matches the known spool-progress race tracked by #1970; CI required remains blocked by that failure.

@ravarora2

ravarora2 commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

🤖 Response to the review and collapse-target question: the smaller toggle target was unintended. In 3cbc353 the summary again spans the full header, and navigation-sidebar.spec.mjs uses its original summary-edge alignment assertion. The new case also clicks the trailing disclosure edge and checks keyboard menu/collapse independence. Removed the chevron auto margin, label growth, and all header/summary wrapping; preserved existing fading; adopted a bounded overlay at cramped enlarged widths; and removed the duplicate theme geometry loop. Replied to each inline comment with its specific change and coverage. All 40 local affected browser checks and hosted browser journey shards pass. Native CI remains blocked by the known spool-progress race tracked by #1970; desktop acceptance and re-review remain pending.

@loganj loganj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Larry: re-review of 3cbc353 (delta from 23d83b9). Source review only; CI is green on this head.

All four of my findings are addressed. The clamped reservation keeps the summary's current padding at 100% and 120%. At 200%/220px it leaves about three control widths for the label and chevron, and the opaque action background covers the trailing text only while the actions show. This is one row at every size, the chevron sits next to the title again, the whole header width toggles the section, and the fade contract stays the same (navigation-group-icons.spec.mjs now also covers 200%). The new spec checks each of these directly.

Non-blocking advice: .sectionActions now paints --surface-panel. When a section is a drop target (.channelSection[data-drop-target], --affordance-panel-hover) and the pointer is over its header, the actions box can show as a separate rectangle on the tinted section. Check it during a channel drag; background: inherit, or a token that follows the section's state, would remove it.

@ravarora2
ravarora2 merged commit 3457ad3 into main Nov 3, 2026
77 of 79 checks passed
@ravarora2
ravarora2 deleted the rarora/AIDA-2338/sidebar-heading-reflow branch November 3, 2026 21:03
dr0uleau added a commit that referenced this pull request Nov 4, 2026
Co-authored-by: Codex <noreply@openai.com>

* origin/main: (327 commits)
  feat(bestie): remove your Bestie, and let remote agents be Bestie after a warning (#2041)
  feat(messages): open the sender's profile from the byline name (#2253)
  feat(native-mcp): one-command launcher and MCP registration for agent driving (#2272)
  perf(composer): stop per-keystroke chip re-renders and reparses in long rich drafts (#2251)
  fix(agents): prevent history from forging prompt requests (#2223)
  Enable automatic key backup in preview releases (#2325)
  feat(read-state): n-blob NIP-RS sync with a 30-day floor, frontier only (#1286)
  feat(huddles): provide read-only huddleStatus for optional consumers (#2327)
  feat(search): show verified names in people search (#2322)
  Fix voice-note sending and restore waveform previews (#2283)
  Preserve Solo queued messages across restarts (#1704)
  feat(plugins): resolve attachment renderers by order, like panels and links (#2284)
  fix(channels): keep muted DM names from becoming bold (#2184)
  fix: command-click opening desktop message links in the browser (#2288)
  AIDA-2390: Explain oversized paste rejection (#2271)
  AIDA-2399: Make failed uploads easy to retry or remove (#2265)
  AIDA-2338: Keep sidebar headings readable at large sizes (#2263)
  chore(plugins): export added-tool types and clarify upload docs (#2292)
  feat(solo): open chats for community viewing (#2107)
  feat(dev): read BUZZ_ settings from .env.local into the desktop launch environment (#2254)
  ...

Signed-off-by: Daniel Rouleau <drouleau@squareup.com>

# Conflicts:
#	docs/browser-testing.md
#	docs/local-artifact-storage.md
#	src/app/services.ts
#	src/features/artifact-storage/service.test.ts
#	src/features/artifact-storage/service.ts
#	src/features/artifact-storage/store.ts
#	src/features/communities/service.ts
#	tests/browser/artifact-storage.spec.mjs
#	tests/integration/browser-ci.test.mjs
wpfleger96 added a commit that referenced this pull request Nov 4, 2026
…w-tab

* origin/main: (21 commits)
  fix: prevent false leave failures after membership updates (#2310)
  feat(bestie): remove your Bestie, and let remote agents be Bestie after a warning (#2041)
  feat(messages): open the sender's profile from the byline name (#2253)
  feat(native-mcp): one-command launcher and MCP registration for agent driving (#2272)
  perf(composer): stop per-keystroke chip re-renders and reparses in long rich drafts (#2251)
  fix(agents): prevent history from forging prompt requests (#2223)
  Enable automatic key backup in preview releases (#2325)
  feat(read-state): n-blob NIP-RS sync with a 30-day floor, frontier only (#1286)
  feat(huddles): provide read-only huddleStatus for optional consumers (#2327)
  feat(search): show verified names in people search (#2322)
  Fix voice-note sending and restore waveform previews (#2283)
  Preserve Solo queued messages across restarts (#1704)
  feat(plugins): resolve attachment renderers by order, like panels and links (#2284)
  fix(channels): keep muted DM names from becoming bold (#2184)
  fix: command-click opening desktop message links in the browser (#2288)
  AIDA-2390: Explain oversized paste rejection (#2271)
  AIDA-2399: Make failed uploads easy to retry or remove (#2265)
  AIDA-2338: Keep sidebar headings readable at large sizes (#2263)
  chore(plugins): export added-tool types and clarify upload docs (#2292)
  feat(solo): open chats for community viewing (#2107)
  ...

# Conflicts:
#	src/features/relay/outbox.ts
arividar pushed a commit to minniai/minni-buzz-app that referenced this pull request Nov 4, 2026
## Why

At 200% interface size, a 220px sidebar can leave no width for
“Channels” or “Direct messages”. The header reserves scaled button space
inside a fixed-width sidebar. Fixes
[AIDA-2338](https://linear.app/squareup/issue/AIDA-2338).

## What

Cap the space reserved for actions so headings retain useful text width.
Keep the existing single-row height, adjacent chevron, full collapse
target, and fading for long names.

## How

Normal widths retain the original action clearance. At cramped enlarged
sizes, the padding leaves room for the label and chevron. Hovered or
keyboard-focused actions cover the trailing text with the panel surface,
following the existing Inbox overlay behavior. Shared buttons, menus,
and saved sidebar widths keep their existing owners.

Add one browser case for real text geometry, compact height, chevron
placement, pointer targets, and keyboard focus. Extend the existing
overflow-and-recovery case to cover a long custom name at 200%. No
existing browser cases are removed. These contracts need a browser
layout engine. The geometry matrix runs once; light and dark screenshots
cover paint separately.

## Risk

At the narrowest enlarged sizes, revealed actions cover the end of a
heading. Long names still fade, and the full accessible names remain
available. Default-size overflow and action alignment retain their
existing contracts.

## Testing

Local Chromium and WebKit fixtures exercised 100%, 120%, 150%, 180%, and
200% interface sizing at 220/260/520px sidebar widths. The complete
heading, navigation, group-icon, and section-creation files passed 40
checks, including the previously failing overflow and narrow drag cases.
Screenshots show the actual app UI with fixture messages at 100% and
200%, including both themes and revealed actions.

Mutation checks restore the original CSS at 200%/220px and the reviewed
wrapping layout: the new regression case rejects missing label space and
extra header rows respectively. The checks pass with the revised CSS.

All hosted browser journey shards passed at `3cbc353e1`. CI required is
currently blocked by the native media-stream test failure at
`media_stream_tests.rs:300`, matching the known spool-progress race
tracked by [block#1970](block#1970). Human
desktop acceptance remains pending. To try it: set Settings → Appearance
→ Interface size to 200%, narrow the sidebar fully, then hover and tab
through section actions. Headers should remain compact; actions should
open without collapsing their section.

Generated with Codex

---------

Signed-off-by: Ravneet Arora <rarora@squareup.com>
Co-authored-by: BuzzPRReviewAssistant1 <a581a7e8115222acdc32ffb14bfce55d48c24478d98e155f8eba126b69b83c7b@buzz.block.builderlab.xyz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants