You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "liuxiaocs7 (via GitHub)" <gi...@apache.org> on 2026/09/10 11:58:37 UTC
[PR] fix(desktop): keep expanded tool calls in flow [maka]
liuxiaocs7 opened a new pull request, #5158:
URL: https://github.com/apache/maka/pull/5158
## Summary
Keep expanded tool-call group and row disclosures in the transcript's normal document flow instead of pinning them over adjacent conversation content. The reasoning disclosure retains its existing sticky header behavior.
The existing narrow grouped-tool Storybook interaction now asserts that the expanded group header computes to `position: static`, covering the regression at the real presentation seam.
Fixes #5157
Refs #4773
## Verification
- `npm run format:check`
- `npm run lint`
- `npm --workspace @maka/desktop run typecheck:stories`
- `npm --workspace @maka/desktop run build-storybook`
- Storybook `Product/Tool Activity/Long Intent Group Narrow`: the regression assertion fails on `main` with `position: sticky` and passes here with `position: static`
- Headless browser check: expanded group header reports `position: static; top: auto; z-index: auto`
Before/after screenshots are attached below.
## AI use
Select exactly one:
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: OpenAI Codex diagnosed the CSS interaction, implemented the scoped selector change, added the Storybook regression assertion, and drafted this issue and pull request under human direction.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): keep expanded tool calls in flow [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on code in PR #5158:
URL: https://github.com/apache/maka/pull/5158#discussion_r3987058283
##########
apps/desktop/src/renderer/styles/chat-message.css:
##########
@@ -192,23 +190,11 @@
box-shadow: 0 var(--border-width-hairline) 0 var(--border-soft);
}
-/* A call detail inside an expanded tool group forms a second, bounded sticky
- tier. The group remains available to collapse the whole run; the lower row
- collapses only the detail currently being read.
- Stock ChatToolCalls uses role=button on call rows (no data-slot patch). */
-.maka-turn
- .maka-tool-activity-card
- > [role="button"][aria-expanded="true"]
- ~ div
- [role="button"][aria-expanded="true"] {
- top: calc(var(--maka-activity-sticky-top) + var(--maka-activity-header-height));
-}
-
/* Astryx clips the animated group body with overflow:hidden. `clip` preserves
- that visual boundary without becoming a scroll ancestor that captures the
- nested sticky row. The inner wrapper is also a grid item: without an explicit
- zero minimum its long, nowrap call preview becomes the track's minimum size
- and stretches every expanded row past the turn instead of ellipsizing it. */
+ that visual boundary. The inner wrapper is also a grid item: without an
+ explicit zero minimum its long, nowrap call preview becomes the track's
+ minimum size and stretches every expanded row past the turn instead of
+ ellipsizing it. */
.maka-turn .maka-tool-activity-card > [role="button"] + div > div {
min-width: 0;
overflow: clip;
Review Comment:
Astryx's own `xb3r6kr` class already sets `overflow: hidden` on this wrapper (checked in the built stylesheet), so `clip` overrides Astryx's value rather than preserving its boundary, and the only reason it did was the nested sticky row this PR removes. I measured group body, wrapper and row geometry under `clip`, `hidden` and `visible` in `Long Intent Group Narrow` — all three identical (wrapper 328×48, `scrollWidth` 328, max row width 328, turn 320). Drop `overflow: clip`, keep `min-width: 0`, and trim the comment to the grid-track reason.
`clip` 是为已删除的嵌套 sticky 行而覆盖 Astryx 的 `hidden`，现在可以删掉，注释只保留 grid 轨道那条理由。
##########
packages/ui/stories/tool-activity.stories.tsx:
##########
@@ -276,6 +276,8 @@ export const LongIntentGroupNarrow: Story = {
);
expect(disclosure).not.toBeNull();
await userEvent.click(disclosure!);
+ await waitFor(() => expect(disclosure).toHaveAttribute('aria-expanded', 'true'));
+ expect(getComputedStyle(disclosure!).position).toBe('static');
Review Comment:
This covers the group header, but the diff also deletes two call-row-specific rules (the `.maka-tool-activity-card [role="button"][aria-expanded="true"]` branch and the second-tier `top` keyed on `> [role="button"][aria-expanded="true"] ~ div [role="button"][aria-expanded="true"]`), and nothing walks that second selector path — a future rule that re-pins rows without re-pinning the group header would land green. The rows in this fixture are already expandable; I confirmed by hand that an expanded row computes `position: static`. Add: click a row, `await waitFor(() => expect(row).toHaveAttribute('aria-expanded', 'true'))`, then assert its `position` is `static`.
再点开一行并断言 `position: static`，以覆盖被删掉的行级第二层规则。
##########
apps/desktop/src/renderer/styles/chat-message.css:
##########
@@ -173,17 +173,15 @@
contain-intrinsic-block-size: auto 32px;
}
-/* Expanded activity headers stay reachable while their own detail is being
- read. Native sticky positioning keeps the header in the transcript flow,
- so it leaves naturally at the card boundary and does not disturb ChatLayout
- scroll anchoring while reasoning or tool output continues to stream. */
-.maka-turn :is(.maka-deep-thinking, .maka-tool-activity-card) {
+/* An expanded reasoning header stays reachable while its own detail is being
+ read. Tool-call disclosures remain in normal flow: pinning both the group
+ header and an expanded call row creates a floating two-tier stack that
+ visually detaches from the transcript around adjacent messages. */
+.maka-turn .maka-deep-thinking {
Review Comment:
`--maka-activity-sticky-top` now has one producer (here) and one consumer (`:186`), and it is also the only thing keeping the sticky treatment off non-reasoning cards — `:184` still matches any `.maka-activity-card-header[aria-expanded="true"]` in a turn, so such a header keeps `position: sticky`, `z-index: 20`, the raised background and the hairline, with only `top` falling back to `auto`. That is a half-applied pin plus a stray stacking context, not a clean opt-out. Put the scope in the selector and drop the variable: delete this rule and make `:184` read `.maka-turn .maka-deep-thinking .maka-activity-card-header[aria-expanded="true"]` with `top: var(--space-2)`.
把作用域写进选择器并删掉这个只有一处生产、一处消费的变量。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): keep expanded tool calls in flow [maka]
Posted by "liuxiaocs7 (via GitHub)" <gi...@apache.org>.
liuxiaocs7 commented on PR #5158:
URL: https://github.com/apache/maka/pull/5158#issuecomment-5632806511
Thanks for the detailed review and verification, @Astro-Han. I addressed the follow-ups in `6178fbaaff`:
- Scoped the sticky styling directly to `.maka-deep-thinking` and removed `--maka-activity-sticky-top`.
- Removed the obsolete `overflow: clip` override while keeping `min-width: 0`.
- Extended the Storybook interaction to expand a call row and assert that both the group header and call-row disclosure use `position: static`. The reasoning header remains sticky.
- Replaced the evidence with before/after captures from the same `Long Intent Group Narrow` story, viewport, expanded state, and scroll position.
I could not upload native PR attachments because this account does not have write access to the upstream repository, so I used persistent GitHub Release Assets instead of branch-backed raw URLs. CI is green, and all three review threads are resolved.
Thanks again for catching these details.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(desktop): keep expanded tool calls in flow [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #5158:
URL: https://github.com/apache/maka/pull/5158
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org