fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix(ui): restore usable scrolling for one-line code blocks - #3170

Merged
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar
Aug 19, 2026
Merged

fix(ui): restore usable scrolling for one-line code blocks#3170
Astro-Han merged 5 commits into
apache:mainfrom
MicroGery:fix/single-line-code-scrollbar

Conversation

@MicroGery

@MicroGeryMicroGery commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • move the copy action into a dedicated CodeBlock toolbar so it no longer covers long single-line content
  • keep the Astryx native overflow viewport and platform scrollbar instead of adding a second scrolling implementation
  • prevent Turn-level pointer capture from stealing native scrollbar gestures or edge-selection autoscroll
  • vertically center one-line code while preserving multiline CodeBlock behavior

Verification

  • npm run rebuild
  • npm --workspace @maka/ui test — 182 passed
  • npm run lint -- --diagnostic-level=error
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • manually compared the latest origin/main behavior with this branch in the desktop GUI

Root cause

Astryx places the copy button absolutely when a plaintext CodeBlock has no header, so the control can cover the end of a long one-line value. Independently, Maka captures pointer gestures on the enclosing Turn for quote selection; that retargets pointer movement away from the nested code viewport and prevents Chromium from continuing native scrollbar or edge-selection scrolling. The fix gives CodeBlock a structural toolbar and lets its native viewport retain ownership of scrolling and selection gestures.

AI use

Select exactly one:

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

Tool(s) and scope: OpenAI Codex assisted with root-cause analysis, implementation, regression tests, rebase, and local verification. The contributor reviewed the resulting changes.

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

@coderabbitai

coderabbitaiBot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fdb6861c-13f1-4800-ae89-c8696a1a5c2b

📥 Commits

Reviewing files that changed from the base of the PR and between dce234b and 46a36de.

📒 Files selected for processing (1)
  • packages/ui/src/__tests__/markdown-body.test.ts
💤 Files with no reviewable changes (1)
  • packages/ui/src/tests/markdown-body.test.ts

Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Problem solved

This PR restores horizontal scrolling for one-line Markdown code blocks.

It moves the copy action into a dedicated CodeBlock toolbar. It preserves the native Astryx scroll viewport and platform scrollbar. It prevents Turn-level pointer capture from blocking scrollbar dragging and selection autoscroll. It vertically centers one-line code without changing multiline behavior.

Source of truth

The PR extends the existing Markdown code-block renderer and message-selection logic. It does not create a parallel rendering path.

The data-maka-code-layout attribute exposes layout state for styling and tests. The preservesNativeSelectionScroll helper adds a targeted exception for Markdown code scrolling regions.

Solution size and complexity

The changes form the smallest coherent solution shown by the supplied summary:

  • markdown-body.tsx classifies code blocks and supplies toolbar structure.
  • styles.css defines toolbar, layout, focus, and horizontal-scroll behavior.
  • use-message-selection-quote.ts limits pointer-capture changes to native code scrolling regions.
  • Unit tests cover layout, copy-control ordering, accessibility, standalone rendering, and selection behavior.
  • The Electron test covers overflow, wheel scrolling, keyboard scrolling, and selection dragging.

The toolbar, layout state, selection exception, and scrollbar behavior address separate regression causes. No implementation or test change can be deleted without weakening behavior or regression coverage based on the supplied diff.

Complexity delta

  • Authorities: The existing Markdown renderer remains authoritative. Astryx remains authoritative for native scrolling and selection.
  • State: The PR adds single-line versus multiline layout state and native-selection eligibility state.
  • Branches: It adds layout classification, localized plaintext collapsibility, and a targeted pointer-capture bypass.
  • Configuration: It adds scrollbar presentation and light/dark styling.
  • Public surface: It adds the exported preservesNativeSelectionScroll function. No other exported entity changes.
  • Test burden: It adds Markdown layout tests, selection-boundary tests, and Electron interaction coverage.

Maintenance complexity increases in the affected UI path. The increase is justified by the separate copy-button, scrollbar, layout, and pointer-capture requirements.

Optional review findings remain follow-up considerations. They are not implementation requirements for this PR.

Validation

The PR reports a rebuild, 182 passing UI tests, linting, and one passing Electron code-scroll test. The review reports green CI and a PASS result with no P0, P1, or P2 issues.

A follow-up commit removes a stale scrollbar implementation assertion from the UI tests. A P3 review request asks for before-and-after screenshots in light and dark themes.

Required-check status is unverified here because no direct check output is available.

Review-relevant risks

  • The PR changes user-visible code-block layout, toolbar placement, focus behavior, and scrollbar presentation. Material UI changes require independent human review under repository policy.
  • The PR changes pointer handling and text-selection behavior in message turns. Material interaction changes require independent human review under repository policy.
  • The PR makes plaintext code blocks collapsible and adds localized accessible titles. Material accessibility and behavior changes require independent human review under repository policy.
  • The PR adds an exported helper. Public contract changes require independent human review under repository policy.

No security, licensing, release, or governance effect was identified in the current diff.

The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

Markdown code blocks now classify single-line and multiline layouts, support horizontal scrolling, preserve native selection scrolling, and expose localized collapsible controls for plaintext blocks. Unit and Playwright tests cover layout, accessibility, wheel, keyboard, and drag interactions.

Changes

Code block scrolling

Layer / File(s)Summary
Code block layout and styling
packages/ui/src/markdown-body.tsx, packages/ui/src/styles.css, packages/ui/src/__tests__/markdown-body.test.ts
Code blocks classify layout, trim trailing empty lines, and pass collapsible titles and thresholds. Styles add toolbar spacing, horizontal scrolling, focus rings, and alignment. Tests cover layout ordering and localized plaintext controls.
Native selection-scroll preservation
packages/ui/src/use-message-selection-quote.ts, packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
Pointer capture skips Markdown code-block scrolling regions. DOM tests distinguish code content from ordinary prose.
Scrolling interaction validation
apps/desktop/e2e/code-scroll.spec.ts
Playwright tests cover overflow metrics, wheel and keyboard scrolling, selection-drag scrolling, and text selection.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk:🔵 Low · up to 46a36

The PR is mergeable with owner awareness of a bounded dependency-notice mismatch: the permitted version range may include versions not covered by the exact-version MIT notice override; no other actionable merge blocker is supplied.

Sequence Diagram(s)

sequenceDiagram
participant MarkdownBody
participant CodeBlockViewport
participant useMessageSelectionQuote
participant User
MarkdownBody->>CodeBlockViewport: render classified code block
User->>CodeBlockViewport: scroll with wheel or keyboard
User->>useMessageSelectionQuote: start pointer selection
useMessageSelectionQuote->>CodeBlockViewport: detect native selection-scroll region
useMessageSelectionQuote-->>User: skip pointer capture
User->>CodeBlockViewport: drag-select and horizontally scroll
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check nameStatusExplanationResolution
Ai Use Disclosure⚠️ WarningThe PR names OpenAI Codex for implementation and regression tests, but only d9898b8 has Generated-by: Codex; later implementation/test commits 15e89b8 and dce234b lack valid trailers.Add standalone Generated-by: Codex trailers to each affected commit and ensure they survive squash or amend. See CONTRIBUTING.md, “Human ownership and AI attribution”.
✅ Passed checks (4 passed)
Check nameStatusExplanation
Description check✅ PassedThe description covers the required summary, verification, AI use, checklist, and behavior-change sections with specific implementation and test details.
Title check✅ PassedThe title clearly and concisely describes the main change: restoring usable scrolling for one-line code blocks.
Linked Issues check✅ PassedCheck skipped because no linked issues were found for this pull request.
Out of Scope Changes check✅ PassedCheck skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Restore horizontal scrolling for single-line Markdown code blocks

🐞 Bug fix🧪 Tests⚙️ Configuration changes🕐 40+ Minutes

Grey Divider

AI Description

• Move CodeBlock copy action into a real toolbar to stop covering long single-line code.
• Re-introduce an external horizontal scrollbar slot via OverlayScrollbars, theme-adaptive.
• Prevent Turn-level pointer capture from breaking scrollbar drags and selection autoscroll.
Diagram

graph TD
D["Selection quote"] --> A["Markdown code"] --> B{{"OverlayScrollbars"}}
C["UI styles"] --> B
E["Markdown tests"] --> A
F["Quote tests"] --> D
G["E2E scroll test"] --> A
H["Notices script"] --> B
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. CSS-only: use native horizontal scrollbars
  • ➕ Avoids adding a new runtime dependency
  • ➕ Less lifecycle/DOM wiring and fewer moving parts
  • ➖ Platform scrollbars can be hidden/overlaid and may be hard to grab
  • ➖ Harder to guarantee a persistent, usable scrollbar affordance in all themes
2. Fully wrap CodeBlock with a custom scroll container
  • ➕ Clear ownership over viewport/content, potentially simpler mental model
  • ➕ Can unify keyboard/selection/scroll behavior into one component
  • ➖ Higher risk to accessibility and selection behavior currently provided by Astryx
  • ➖ More invasive change; likely to break other CodeBlock variants
3. Customize/extend Astryx CodeBlock (upstream-style fix)
  • ➕ Keeps scroll/copy behavior centralized in the component library
  • ➕ Potentially benefits all consumers of Astryx CodeBlock
  • ➖ May require upstream changes/release coordination
  • ➖ Less flexibility for Maka-specific slot placement and interaction constraints

Recommendation: The chosen approach (OverlayScrollbars only for rendering a dedicated horizontal scrollbar while keeping Astryx’s native viewport/content) is a good compromise: it fixes usability for single-line code without regressing selection/keyboard behavior. The added dependency is justified by the UX requirement for a consistently usable scrollbar, and the PR mitigates risk with explicit gesture exclusions plus unit and e2e coverage.

Files changed (10) +398 / -3

Bug fix (3) +190 / -2
markdown-body.tsxAttach OverlayScrollbars slot for single-line code and force toolbar header+104/-2

Attach OverlayScrollbars slot for single-line code and force toolbar header

• Detects single-line code fences and marks them with a data-maka-code-layout attribute. Forces Astryx CodeBlock to render its header toolbar (empty title) so the copy button no longer overlays content, and mounts an OverlayScrollbars instance that hides native horizontal scrollbars while rendering a themed horizontal scrollbar into a dedicated slot and preserving keyboard arrow scrolling.

packages/ui/src/markdown-body.tsx

styles.cssStyle single-line code layout, toolbar header, and scrollbar slot+67/-0

Style single-line code layout, toolbar header, and scrollbar slot

• Imports OverlayScrollbars base CSS and adds Maka-specific styling: a consistent CodeBlock header toolbar, corrected body spacing, hidden native scrollbars for single-line layouts, and an absolutely positioned scrollbar slot that only appears when usable. Also adds focus styling to avoid double outlines in list-item contexts.

packages/ui/src/styles.css

use-message-selection-quote.tsSkip Turn-level pointer capture for code viewport and scrollbar gestures+19/-0

Skip Turn-level pointer capture for code viewport and scrollbar gestures

• Introduces a target matcher to detect Markdown code viewport/scrollbar interactions that must keep native pointer-driven scrolling and selection-edge autoscroll. Uses this to bypass Turn-level pointer capture so scrollbar drags and selection gestures aren’t interrupted while still allowing quote creation from selection changes.

packages/ui/src/use-message-selection-quote.ts

Tests (3) +167 / -1
code-scroll.spec.tsAdd Electron e2e coverage for single-line code horizontal scrolling+108/-0

Add Electron e2e coverage for single-line code horizontal scrolling

• Introduces an end-to-end spec that asserts long single-line Markdown code blocks can scroll horizontally via scrollbar drag, track click, keyboard arrows, and selection-edge autoscroll. Also verifies scrollbar presence only when needed and checks layout insets for centered single-line rendering.

apps/desktop/e2e/code-scroll.spec.ts

markdown-body.test.tsTest CodeBlock toolbar ordering and single-line layout switching+30/-0

Test CodeBlock toolbar ordering and single-line layout switching

• Adds unit assertions that the copy control lives in the CodeBlock header toolbar above the scroll viewport for long single-line code. Also verifies multiline code does not receive the single-line scrollbar slot while retaining the header/copy button.

packages/ui/src/tests/markdown-body.test.ts

message-selection-quote-boundary.test.tsTest selection-quote gesture exemptions for Markdown code scrolling targets+29/-1

Test selection-quote gesture exemptions for Markdown code scrolling targets

• Adds linkedom-based DOM tests to ensure elements inside the Markdown code viewport and scrollbar slot are treated as preserving native selection/scroll behavior. Verifies non-code prose does not get the exemption.

packages/ui/src/tests/message-selection-quote-boundary.test.ts

Documentation (1) +30 / -0
THIRD_PARTY_NOTICES.txtAdd pinned MIT notice block for overlayscrollbars@2.16.0+30/-0

Add pinned MIT notice block for overlayscrollbars@2.16.0

• Adds a version-pinned license text override entry for OverlayScrollbars 2.16.0, ensuring the MIT notice is included verbatim for compliance when the tarball lacks the repository LICENSE file.

apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt

Other (3) +11 / -0
package-lock.jsonLock OverlayScrollbars dependency at 2.16.0+7/-0

Lock OverlayScrollbars dependency at 2.16.0

• Adds overlayscrollbars@2.16.0 to the lockfile and wires it into the UI package dependency graph. Ensures deterministic installs for the newly introduced scrollbar library.

package-lock.json

package.jsonAdd overlayscrollbars dependency to @maka/ui+1/-0

Add overlayscrollbars dependency to @maka/ui

• Declares overlayscrollbars ^2.16.0 as a runtime dependency to support custom scrollbars for single-line Markdown code blocks.

packages/ui/package.json

generate-third-party-notices.mjsPin MIT copyright override for overlayscrollbars@2.16.0+3/-0

Pin MIT copyright override for overlayscrollbars@2.16.0

• Adds a version-specific MIT copyright override entry for overlayscrollbars@2.16.0 so the notices generator emits the correct license header and forces re-validation on version bumps.

scripts/generate-third-party-notices.mjs

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/ui/src/markdown-body.tsx (1)

20-20: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Remove the unnecessary ClickScrollPlugin.

clickScroll: 'instant' does not require this plugin. Remove its import and global registration to reduce bundle size and initialization work.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a1c5e907-e619-455e-a0db-6127feace5d0

📥 Commits

Reviewing files that changed from the base of the PR and between 2de6d6d and 18261a0.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (9)
  • apps/desktop/e2e/code-scroll.spec.ts
  • apps/desktop/resources/licenses/npm/THIRD_PARTY_NOTICES.txt
  • packages/ui/package.json
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/__tests__/message-selection-quote-boundary.test.ts
  • packages/ui/src/markdown-body.tsx
  • packages/ui/src/styles.css
  • packages/ui/src/use-message-selection-quote.ts
  • scripts/generate-third-party-notices.mjs

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.

Comment threadpackages/ui/package.json Outdated
@qodo-code-review

qodo-code-reviewBot commented Aug 17, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0)📘 Rule violations (0)📜 Skill insights (0)

Grey Divider


Remediation recommended

1. Flaky scrollbar geometry waits✓ Resolved🐞 Bug☼ Reliability
Description
The new code-scroll E2E reads scrollbar/thumb bounding boxes immediately after only asserting the
scrollbar is visible, and uses a fixed 250ms delay to wait for selection-driven autoscroll. On
slower/variable CI timing this can yield null/stale geometry or sample scrollLeft too early, making
the test nondeterministic.
Code

apps/desktop/e2e/code-scroll.spec.ts[R59-62]

+ await expect(scrollbar).toBeVisible();+ await expect(scrollbar).not.toHaveClass(/os-scrollbar-unusable/);+ const trackBox = await scrollbar.boundingBox();+ const thumbBox = await thumb.boundingBox();
Relevance

●●● Strong

Accepted precedents require explicit geometry synchronization; this test has the same boundingBox
and timing flake pattern.

PR-#3160
PR-#2191

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The test currently only asserts the scrollbar container visibility before sampling geometry, and it
uses a fixed sleep before verifying selection autoscroll. Prior accepted flake guidance in this repo
recommends explicit visibility synchronization before boundingBox-based geometry assertions.

apps/desktop/e2e/code-scroll.spec.ts[54-102]
PR-#3160

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
The new Playwright test can be timing-sensitive because it:
- calls `thumb.boundingBox()` without first ensuring the thumb itself is visible/laid out,
- uses a fixed `waitForTimeout(250)` before asserting selection autoscroll happened.
This can cause intermittent failures in CI (null/stale bounding boxes or reading `scrollLeft` before it changes).
## Issue Context
OverlayScrollbars DOM/geometry can settle a tick after the container becomes visible, and selection-driven autoscroll timing varies across machines.
## Fix Focus Areas
- apps/desktop/e2e/code-scroll.spec.ts[54-102]
## Suggested changes
- Add `await expect(thumb).toBeVisible()` before reading `thumb.boundingBox()`.
- Optionally replace direct boundingBox reads with `expect.poll` to wait until both `trackBox` and `thumbBox` are non-null and have sensible dimensions.
- Replace `await page.waitForTimeout(250)` with a bounded poll, e.g. `await expect.poll(async () => viewport.evaluate(el => (el as HTMLElement).scrollLeft)).toBeGreaterThan(0)` (and/or poll for selection length), so the test waits exactly as long as needed (up to a timeout) rather than sleeping a fixed amount.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Web pages:
+6 more
Review mode: ⚖️ Balanced: Downgraded extended -> standard: change is below the extended eligibility bar (hunks 16/18, lines 401/200; both must reach the floor). Router rationale: This is a bug-dense UI behavior change spanning rendering, third-party scrollbar integration, CSS layout, pointer/selection gesture handling, tests, and licensing/build metadata, with multiple independent interaction paths that merit redundant review.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the broken scrolling is real and the diagnosis is precise (the absolute-positioned copy button covers the top-right of untitled code blocks, the turn-level setPointerCapture in the quote hook swallows scrollbar dragging and cross-edge selection autoscroll, and macOS's hidden overlay scrollbar makes the row unreachable), and the cure is scoped correctly: :has()/title="" toolbar structure, pointer-capture exemption, and OverlayScrollbars in custom-elements mode (keeping the native viewport so selection/keyboard scrolling aren't lost) are three distinct fixes for three distinct causes, with the e2e directly exercising the four previously-broken interactions (scrollbar drag, track click, keyboard, selection autoscroll) — verified against the Astryx source that the theme mapping is right (os-theme-light on dark app), that vertical wheel still chains to the chat scroller, and that quote-from-code survives (selectionchange path untouched). CI is green.

Conclusion: PASS — no P0/P1/P2.

P3-1 (merge gate): the PR has no before/after screenshots. This visually changes every code block (plain-text blocks gain a 32px bordered toolbar, single-line blocks grow to ~72px, language-block content shifts down as the Astryx -spacing-2 pull-up is neutralized, long lines gain a persistent scrollbar). Per the repo gate, UI changes need before/after screenshots — please add light/dark pairs.

P3 (optional): title="" also enables the structural header for plain-text blocks, which with the always-passed isCollapsible makes plain-text blocks ≥10 lines collapsible for the first time — a new interaction not mentioned in the PR (acceptable for consistency, but please confirm it's intended); the scrollbar slot overlays a 10px strip at the bottom (z-index:1) and a pointerdown there triggers clickScroll instead of text selection/copy — worth a CJK long-command screenshot to confirm; OverlayScrollbars (~30KB) buys "persistent themed scrollbar + touch/pen drag" over the native thin-scrollbar path — defensible as a product choice but the tradeoff isn't stated in the PR; the e2e doesn't verify the key nested-scrolling promise (hovering a code block doesn't break the outer chat vertical scroll) or toolbar copy-button clickability — per convention, test gaps are P3; the custom ArrowLeft/Right keydown handler in markdown-body.tsx is redundant with the native keyboard scroll on the tabIndex=0 viewport and unconditionally preventDefaults (also swallowing selection-collapse+scroll when a selection exists) — either delete it or only intercept when there's remaining scroll in that direction.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the spacer/ChatLayout geometry, the pointer-capture path, and verified OS 2.16 / Astryx 0.4.0 source. P3-1 is from the PR having no screenshots. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1/P2)。问题三源精确定位:绝对定位复制按钮盖住无标题块右上角 + turn 级 setPointerCapture 吞掉滚动条拖拽/跨边缘选区 autoscroll + macOS 隐藏 overlay scrollbar 使长行不可达;修复各就各位(工具栏化+指针豁免+OverlayScrollbars custom-elements 模式保留原生 viewport 保选区/键盘滚动),e2e 直接验证四个曾被破坏的交互(滚动条拖拽/轨道点击/键盘/选区 autoscroll),主题映射/垂直滚不被吞/quote-from-code 存活均已核对。CI 全绿。P3-1(合并 gate):无 before/after 截图——本 PR 视觉上改变每个代码块(纯文本块新增 32px 带边框工具栏、单行块增高至 ~72px、语言块内容因中和 Astryx -spacing-2 上拉而下移、长行新增持久滚动条),按仓库规则需要 light/dark 前后截图。P3(可选):title="" 使纯文本块首次拥有 header,配合恒传 isCollapsible 使 ≥10 行纯文本块可变折叠——新交互 PR 未提及,需确认是有意;滚动条 slot 覆盖底部 10px 条带(z-index:1),该处 pointerdown 触发 clickScroll 而非文本选区/复制——建议用 CJK 长命令截图核实;OverlayScrollbars(~30KB)换持久主题滚动条+触控拖拽是可选产品决策但 PR 未陈述取舍;e2e 未验证嵌套滚动核心承诺(悬停代码块不破坏外层 chat 垂直滚动)与工具栏复制按钮可点;markdown-body.tsx 自定义 ArrowLeft/Right handler 与 tabIndex=0 viewport 的原生键盘滚动冗余且无条件 preventDefault(含提示词选区折叠+滚动),建议删除或仅在仍有滚动余量时拦截。

@Astro-HanAstro-Han 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.

The scrolling solution is well targeted: it preserves Astryx's native focusable viewport, isolates OverlayScrollbars to one-line blocks, and covers pointer, keyboard, selection, thumb, and track paths without building a parallel code-block component. Two boundary details remain.

From first principles, a control needs a name and a shipped dependency needs a stable reviewed license identity. For headerless plaintext, either do not expose collapse or provide a localized accessible name; for OverlayScrollbars, make the version/notice contract explicit rather than combining a moving caret range with an exact-version override.

Review performed with two Codex reviewer agents and DeepSeek V4 Flash as advisory tools; I verified the findings against the latest head and live green CI.

中文评论

滚动方案很克制:保留 Astryx 原生可聚焦 viewport,只对单行 block 隔离使用 OverlayScrollbars,并覆盖 pointer、keyboard、selection、thumb 和 track 路径,没有新建并行 code-block 组件。仍有两个边界问题。

从第一性原理看,control 必须有名称,发布依赖必须有稳定、已审查的 license identity。对无 header 的 plaintext,要么不暴露 collapse,要么提供本地化 accessible name;对 OverlayScrollbars,应显式固定 version/notice 契约,而不是把移动的 caret range 与精确版本 override 组合。

本次审查使用了两位 Codex reviewer agents 与 DeepSeek V4 Flash 作为辅助工具;我已依据最新 head 和实时绿色 CI 复核问题。

Comment threadpackages/ui/src/markdown-body.tsx Outdated
Comment threadpackages/ui/package.json Outdated
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 18261a0 to 0486a93CompareAugust 18, 2026 14:25
@MicroGery
MicroGeryforce-pushed the fix/single-line-code-scrollbar branch from 0486a93 to d9898b8CompareAugust 18, 2026 14:27

@Astro-HanAstro-Han 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.

The move back to native scrolling removes the earlier dependency/license drift, and polling is better than the fixed geometry timeout. The latest required E2E still fails because the new vertical-centering assertion measures glyph ink bounds rather than the layout box it intends to constrain.

Following Occam's razor, delete this low-quality metric or replace it with the actual line wrapper/viewport layout box after visual verification; changing production spacing to satisfy font ascent/descent asymmetry would optimize for the test rather than the user-visible requirement. The existing accessible-name finding on plaintext collapse is already public, so I am not duplicating it here.

Reviewed with Codex using two independent review passes and DeepSeek V4 Flash as an external adversarial pass; I verified the metric and live E2E failure against this exact head.

中文

回退到原生滚动已消除之前的依赖/许可证漂移,polling 也优于固定等待。但最新 required E2E 仍失败,因为新的垂直居中断言测量的是 glyph ink bounds,而不是它真正想约束的 layout box。

按奥卡姆剃刀,应删除这个低质量指标,或经视觉核验后改测真实 line wrapper/viewport box;不要为了字体 ascent/descent 不对称去修改生产布局。plaintext collapse 的 accessible name 问题已有公开评论,这里不重复。

本次由 Codex 两轮独立审查,并使用 DeepSeek V4 Flash 做外部对抗审查;我核对了当前 head 和 E2E 失败。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated

@coderabbitaicoderabbitaiBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🔇 Additional comments (3)
packages/ui/src/markdown-body.tsx (2)

38-38: LGTM!

Also applies to: 191-210


179-179: 🩺 Stability & Availability

⚠️Unverified finding
Sandbox verification was unavailable.

Verify the new locale-provider requirement.

useUiLocale() now runs for every MarkdownCode, before the Mermaid early return. The hook throws when MarkdownBody renders outside LocaleProvider. This can make Mermaid and standalone Markdown consumers fail at runtime.

Confirm that every MarkdownBody caller and test is inside LocaleProvider. If not, reuse the closest existing provider boundary before adding fallback state.

Disposition: follow-up.

As per path instructions: report only concrete, reproducible risks and reuse the closest existing seam before adding new behavior.

packages/ui/src/__tests__/markdown-body.test.ts (1)

48-60: LGTM!


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a63974d9-717e-4632-bfce-0837d773c767

📥 Commits

Reviewing files that changed from the base of the PR and between 0486a93 and 15e89b8.

📒 Files selected for processing (2)
  • packages/ui/src/__tests__/markdown-body.test.ts
  • packages/ui/src/markdown-body.tsx

Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts

@Astro-HanAstro-Han 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.

The production scrolling seam is mostly right: Astryx keeps ownership of the native viewport, and the pointer-capture exception is narrowly scoped. The current head is not ready because its new required E2E asserts font glyph-ink symmetry rather than the layout box the CSS centers, and that exact assertion is failing on Linux CI. I also found a public-component compatibility regression from making useUiLocale() unconditional for every code block.

AI-assisted review: Codex coordinated two independent reviewer passes and an OpenCode Go DeepSeek V4 Flash high-effort adversarial pass. I verified exact head 15e89b85f99ddd27f3d3477cf2e2098ad2d4d1e4, Astryx/CSS ownership, the public export and locale hook, and the failing required E2E. No local tests were run.

中文审查

生产滚动 seam 基本正确:原生 viewport 仍由 Astryx 管理,pointer-capture 豁免范围也足够窄。当前不能合并,因为新增 required E2E 测量的是字体 glyph ink 对称性,不是 CSS 居中的 layout box;该断言正在 Linux CI 失败。另外,所有代码块无条件调用 useUiLocale(),给公共组件新增了未声明的 Provider 硬依赖。

本次为 AI 辅助审查,已核验精确 head、Astryx/CSS 责任边界、公共导出、locale hook 和失败的 required E2E;未运行本地测试。

Comment threadapps/desktop/e2e/code-scroll.spec.ts Outdated
Comment threadpackages/ui/src/markdown-body.tsx Outdated

@Astro-HanAstro-Han 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 is now a focused fix at the existing Astryx seam: the copy control remains in the native CodeBlock toolbar, the one-line body keeps the native horizontal viewport and keyboard/selection behavior, and only pointer gestures originating inside that viewport bypass Turn-level pointer capture. The locale/provider and layout-metric regressions from earlier heads are resolved.

I found no reproducible correctness issue on this head. All current required checks are green. The inline P3 is only an Occam cleanup and does not block approval.

AI-assisted review disclosure: Codex performed the final review using two independent reviewer passes and OpenCode Go DeepSeek V4 Flash (high effort) as an adversarial advisory pass. I verified the Astryx DOM seam, selection/scroll boundary, accessibility name, exact head dce234bd0, and live CI.

中文评论

当前实现已经收敛到 Astryx 的既有扩展点:copy control 留在原生 CodeBlock toolbar;单行代码继续使用原生横向 viewport、键盘和文本选择;只有从该 viewport 发起的 pointer gesture 才跳过 Turn 层 pointer capture。此前 head 的 locale/provider 与布局度量问题都已解决。

当前 head 未发现可复现的正确性问题,所有必需检查均已通过。行内 P3 仅是奥卡姆剃刀式清理,不阻塞 approve。

AI 辅助审查说明:Codex 使用两轮独立 reviewer 审查,并以 OpenCode Go DeepSeek V4 Flash(high effort)进行对抗性辅助审查;我已核对 Astryx DOM seam、selection/scroll 边界、无障碍名称、精确 head dce234bd0 与实时 CI。

Comment threadpackages/ui/src/__tests__/markdown-body.test.ts Outdated
@jackwener

Copy link
Copy Markdown
Member

Refreshed this branch onto current main@0ef1c55b5 and pushed head 46a36deeb.

The product change is unchanged. The only review follow-up is the approved P3 cleanup: I removed the two negative assertions coupled to the abandoned maka-markdown-code-scrollbar-slot implementation name. Positive toolbar/viewport coverage and the behavioral E2E remain.

Fresh verification on this exact head:

  • npm --workspace @maka/ui test — 188 passed
  • npm --workspace @maka/desktop run e2e -- e2e/code-scroll.spec.ts — 1 passed
  • all required hosted checks green

@Astro-Han, please re-review the refreshed exact head so the independent human-review gate can close.

@Astro-HanAstro-Han 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.

The refreshed head 46a36de preserves the previously reviewed production behavior and applies only the agreed Occam cleanup: the two assertions coupled to the abandoned scrollbar implementation name are removed. I rechecked the exact delta from the approved head, the current PR diff, prior findings, and live required checks. The accessibility, standalone-provider, layout-metric, native-scroll, pointer-selection, and deterministic-E2E boundaries remain resolved.

AI-assisted review disclosure: Codex verified the refreshed exact head and current GitHub state; the human maintainer reviewed the final result and authorized approval and merge.

中文评论

刷新后的 46a36de 保留了此前已审查的生产行为,只执行了已同意的奥卡姆剃刀式清理:删除两条与废弃 scrollbar 实现名耦合的断言。我重新核对了相对已批准 head 的精确变化、当前 PR diff、既有 findings 和实时 required checks。无障碍名称、standalone provider、布局度量、原生滚动、pointer selection 与确定性 E2E 边界仍然全部解决。

AI 辅助审查说明:Codex 核验了刷新后的精确 head 和当前 GitHub 状态;人工维护者审阅最终结果并明确授权 approve 与 merge。

@Astro-Han
Astro-Han merged commit 69ae660 into apache:mainAug 19, 2026
12 checks passed
Sign up for freeto 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

@MicroGery@Astro-Han@jackwener