Skip to content

fix(desktop): preserve workbar tab scrollbar behavior - #5204

Merged
me2seeks merged 1 commit into
apache:mainfrom
colaforniaw:fix/workbar-tab-scrollbar-cleanup
Sep 13, 2026
Merged

fix(desktop): preserve workbar tab scrollbar behavior#5204
me2seeks merged 1 commit into
apache:mainfrom
colaforniaw:fix/workbar-tab-scrollbar-cleanup

Conversation

@colaforniaw

@colaforniaw colaforniaw commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

The app's global scrollbar default was a * rule in layer(components). A universal selector is an element-level declaration, so it competes by cascade layer — and components out-ranks astryx-components. That silently overrode the scrollbar-width: none Astryx's TabList sets on its overflow strip, re-showing a persistent bar under the Session Workbar tabs and nudging them off vertical center (a classic bar takes layout height).

Keep the universal scrollbar default, because scrollbar-width is not inherited and nested app scrollports must remain thin, but move the standard properties and the existing WebKit capsule recipe into the existing @layer base block in maka-tokens.css. The later Astryx and product component layers can then own exceptions on their elements: the TabList strip's none wins while ordinary nested scrollports retain the app-wide thin default. Keeping the WebKit rules preserves the established capsule thumb, transparent gutter, and hover treatment; this PR changes their cascade layer, not the app's scrollbar visual baseline.

Refs #2538

82be2cfc94b13d30ca1f173cebd7d2e3

Verification

  • npm run lint, npm run format:check, npm --workspace @maka/desktop run typecheck, npm --workspace @maka/desktop run check:architecture, and npm run check:asf-headers pass.
  • The Storybook build and targeted workbar smoke pass. The story asserts both sides of the ownership contract: the TabList overflow strip resolves to scrollbar-width: none, while an ordinary nested review scrollport resolves to thin.
  • Moving the universal rule back into layer(components) makes the TabList assertion fail (expected 'thin' to be 'none'), so the original regression cannot return silently.
  • product-shell-official-appshell--tail-prefetches-history-until-the-band-is-full fails in the same full-smoke run on this branch and on the base commit (pre-existing, unrelated).

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Maka — root-caused the regression, wrote the fix and the story assertion, and drafted this description.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/S Under 100 readable lines label Sep 11, 2026
@colaforniaw
colaforniaw marked this pull request as draft September 11, 2026 17:08
@colaforniaw
colaforniaw force-pushed the fix/workbar-tab-scrollbar-cleanup branch 2 times, most recently from 435b5ef to 6b783ba Compare September 11, 2026 17:13
@colaforniaw
colaforniaw marked this pull request as ready for review September 11, 2026 17:24
@colaforniaw
colaforniaw force-pushed the fix/workbar-tab-scrollbar-cleanup branch 2 times, most recently from 673d5be to 8ba258b Compare September 11, 2026 18:09

@me2seeks me2seeks 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.

PR 5204 Review

结论

CHANGES_REQUESTED
层机制修复本身正确且最小:把 universal scrollbar-width/scrollbar-colorlayer(components)base.cssstyles.css:35layer(components) 导入)移入 @layer basemaka-tokens.css:1228cascade-layers.css:65 声明 base < astryx-components < components),Astryx TabList 的 scrollbar-width: none 因此重新胜出;story 测试同时断言两侧(strip=none、review panel=thin),契约完整。

发现

[P2] 同一 diff 里还删掉了整套 webkit 胶囊滚动条样式(33 行),但 PR 描述完全未提,这是全 app 的视觉变化

  • 文件: apps/desktop/src/renderer/styles/base.css(删除的 *::-webkit-scrollbar* 5 条规则)
  • 问题:删除的注释写明"Both reference systems converged on the same recipe: a slim pill inset in a transparent gutter"——即胶囊 thumb 是有意的设计基线。而 Desktop 渲染器是 Electron(Chromium),所有平台用户的滚动条都会从自定义胶囊回退到 UA 默认细条(scrollbar-color 在 Chromium 下仍着色,但 2px 内缩胶囊 + hover 加深消失)。这不是修 bug 所必需:TabList 复现条的问题是 scrollbar-width 的层冲突,把 webkit 规则一并移入 @layer base 就能同时保住两者;当前是"移了 width/color、删了 webkit",新旧行为不一致。
  • 建议(二选一,请在 PR 中说明决策):
    1. 恢复 5 条 webkit 规则,与 scrollbar-width/scrollbar-color 一起放进 @layer base(保设计基线,改动同样最小);或
    2. 确认胶囊样式是有意退役——那请更新 PR 描述(当前只说"move it into @layer base"和"remove the old :root workaround",未提 webkit 规则删除),并确认 Storybook 视觉基线已更新。

[P3] 描述里"remove the old :root workaround so the default has one owner"指向的规则在本 diff 和当前树上都不存在

  • 文件: PR 描述 / apps/desktop/src/renderer/styles/base.css
  • 问题:全仓库 grep 不到任何 :root + scrollbar 的规则(diff 前后都没有)。这句话可能是相对更早 base 写的过时描述,或在描述另一个已被合入的改动。
  • 建议:把这句改成实际发生的事(若 P2 选方案 2,正好并入 webkit 退役的说明)。

验证

  • 层序cascade-layers.css:65 @layer reset, theme, base, astryx-components, astryx-tokens, components; → base 低于 astryx-components/components,新默认让位成立。
  • 例外规则全部保赢prompt-rail.css:66(none)、workhub.css:253/257composer.css:540terminal.css:48runtime-host.css:75 都在 layer(components) 或无层(无层恒胜层),不会被 base 默认覆盖。
  • 修复面base.cssstyles.css:35layer(components) 导入,正是原 bug 中 out-rank astryx-components 的来源;移入 base 后该冲突消除。
  • story 测试playwithin/waitFor/expect 均来自已导入的 storybook/test(:22);断言 strip 的 scrollbarWidth === 'none' 与 review panel 的 'thin',双向覆盖。
  • 无残留:renderer 下唯一幸存的 webkit 规则是 prompt-rail.css:67width: 0,与本 PR 无关且保赢)。

@colaforniaw
colaforniaw force-pushed the fix/workbar-tab-scrollbar-cleanup branch from 8ba258b to a5be1da Compare September 12, 2026 02:15
@colaforniaw

Copy link
Copy Markdown
Contributor Author

Addressed:

  • Restored the WebKit capsule scrollbar styles.
  • Moved them with the standard scrollbar rules into @layer base.
  • Updated the PR description and removed the outdated :root wording.

Lint, format, typecheck, Storybook build, and the targeted smoke all pass. The story still verifies strip=none and review panel=thin.

@me2seeks me2seeks 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.

This change correctly restores the intended cascade boundary for the global scrollbar default. The standard properties and WebKit capsule recipe now share the lower base layer, so Astryx and product components can own element-level exceptions. The Session Workbar TabList therefore keeps its intentional scrollbar-width: none, while ordinary nested scrollports retain the app-wide thin default.

I verified the behavior beyond the computed-style assertion: the production Storybook build and this story's play function pass; the rendered tab strip overflows (283px content in 236px) with a zero horizontal scrollbar gutter; and a Chromium cascade experiment confirms that higher-layer scrollbar-width: none suppresses the gutter even with the low-layer WebKit recipe present. The visual capsule treatment remains intact for ordinary scrollports.

The three-way merge against current main applies only this PR's intended CSS move and regression test; it preserves the later Workbar story-service updates already on main. Local format, renderer architecture, rebuilt Storybook typecheck, Storybook build, targeted smoke, and diff-whitespace checks pass. I found no remaining review findings.

中文

该改动正确恢复了全局滚动条默认样式应有的层叠边界。标准属性和 WebKit 胶囊样式现在共同位于较低优先级的 base layer,因此 Astryx 和产品组件能够拥有各自元素级的例外。Session Workbar TabList 保持它有意设置的 scrollbar-width: none,普通嵌套滚动区仍使用全局的 thin 默认值。

我不只检查了计算属性:生产 Storybook build 和本故事的 play 都通过;渲染后的 tab strip 确实溢出(内容 283px、可见 236px),但水平滚动条 gutter 为 0;独立 Chromium 层叠实验也确认,在较低层仍保留 WebKit 胶囊样式时,较高层的 scrollbar-width: none 依然会去掉 gutter。普通滚动区的胶囊视觉样式未丢失。

针对当前 main 的三方合并只应用本 PR 的 CSS 移动和回归测试,并保留 main 上较新的 Workbar story-service 更新。本地 format、renderer architecture、重建依赖后的 Storybook typecheck、Storybook build、目标 smoke 与 diff 空白检查均通过。没有剩余 review 发现。

@me2seeks
me2seeks merged commit fe7a989 into apache:main Sep 13, 2026
1 check passed
@colaforniaw
colaforniaw deleted the fix/workbar-tab-scrollbar-cleanup branch September 13, 2026 06:22
Shouly pushed a commit to Shouly/maka that referenced this pull request Sep 13, 2026
…f0a5d)

Eight upstream commits. One reaches the new renderer's contracts: apache#4878
returns expected failures as codes across IPC — the five Session setters
(`setPermissionMode`, `setCollaborationMode`, `setOrchestrationMode`,
`setModelConfiguration`, `setThinkingLevel`) answer
`DesktopSessionUpdateResult` instead of throwing, `abandonPlanProposal`
answers `PlanControlIpcResult` like its siblings, `send` / `submitMessage`
gain an `attachment_blocked` refusal, and the attachment guard throws a typed
`AttachmentIngestBlockedError` in place of the `attachment_ingest:<code>`
message token. Also in: apache#5216 makes HTML artifacts directly openable
(`app.showArtifactInFolder` beside `openArtifactPath`, which now hands an
HTML artifact to the default app; `isArtifactUserVisible` admits HTML tool
results), apache#5198 unifies WorkHub conversation identity and Host-owned choices
(an Astryx `ChoicePanel` in packages/ui, `keyboardHint` copy, a
`preserveFocus` scroll target), apache#5249's skill picker fix
(`selectedSkillIds` in `chat-input-behavior`), apache#4815 admits structured-only
Messages (`hasMeaningfulMessageContent` in core), apache#4862's ACP live session
lifecycle in the CLI, apache#5204's workbar tab scrollbar css and apache#5180's wider
locale hygiene gate.

Resolution per the sync policy: conflicts under the old renderer's trees,
packages/ui's deleted components, stories, e2e specs and the main tests that
import them stay deleted, and upstream's new files there are dropped
(`features/workhub/model/workspace-name.ts`, packages/ui's `choice-panel.tsx`
and its `index.ts` export, the `styles/base.css` / `workhub.css` /
`maka-tokens.css` edits, `expected-error-presentation.test.ts` and the WorkHub
main tests). git's rename pairing had put upstream's
`features/session-settings/ports.ts`, `features/workhub/testing.ts` and
`platform/desktop/create-session-settings-services.ts` into
`bridge/e2e-fixture.ts`, `components/ui/skeleton.tsx` and
`lib/ported/display-frame-scheduler.ts`; all three keep ours. The renderer
architecture ledger keeps ours, rewritten with `--write`. The e2e budget and
`transcript-scroll-cost.spec.ts` keep ours; upstream's new
`expected-failure-feedback.spec.ts` is trimmed to its second case (the IPC
round trip of the setting and Plan codes), the first needing the WorkHub
surface this build does not ship.

Re-implemented for the new contracts:
- `bridge/sessions.ts` unwraps every update result and rethrows a refusal as
  `ExpectedOperationError` (new `bridge/expected-operation-error.ts`, ported
  from upstream's `operation-diagnostics.ts`), so the turn actions store and
  every caller keep awaiting a summary. `localizedShellErrorMessage` renders
  the code through the new `updateFailures` copy on every surface and
  `AttachmentIngestBlockedError` by its `code`; `sessionSettingFailureCopy`
  is upstream's.
- `ChatInput` routes an `attachment_blocked` refusal through upstream's
  `showSubmissionFeedback` and keeps the draft with the ingest reason;
  `showSkillInvocationFeedback` stays exported for the partial-success toast.
- `FilesTab` reveals through `showArtifactInFolder`; an HTML row shows "View
  in Maka", its menu offers "Open in Default App" first, and its preview's
  external action opens rather than reveals. `artifact-copy` gains
  `viewInMaka` / `openInDefaultApp` in three locales.
- `TipTapEditor` hides Skills already in the draft from the picker (apache#5249);
  the chips are atoms here, so the set is read from the document rather than
  from the `/skill:x` text upstream scans.
- `composer-state.test.ts` asserts the typed preflight error.

packages/ui: `use-chat-scroll.ts` merged cleanly (`preserveFocus`) on top of
our `holdTurn` extensions; `conversation-copy.ts` and `chat-input-behavior.ts`
take upstream's additions. apache#5217's live-turn buffer stays out as before.

The compatible-change declaration is unchanged this round. The release
checklist's baseline note records that the eight commits add no unlisted
renderer surface.

Gates: build:test + build:renderer, typecheck, biome lint and format, locale
hygiene (the widened apache#5180 gate), ASF headers, renderer architecture ledger
(rewritten with `--write`), e2e budget, third-party notices, knip (39 unused
files, unchanged from the twelfth sync), workspace dist tests (desktop 3484
of 3498 with 14 skipped, every other workspace green), Electron smoke (44
checks, no renderer errors), core-dialogue smoke, streaming-switch smoke, and
the trimmed `expected-failure-feedback` e2e case against the real preload.
`packages/runtime` `model-adapter-onerror` fails on this machine before and
after, as in the eleventh and twelfth syncs.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/S Under 100 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants