fix: quiet the task-agent footer, and give it real design-system components#3602
Merged
Conversation
…nt, dark footer wash Closes the design-system gaps the task-agent footer panel kept returning to (lotti3-1fn.8). Each was flagged by 2+ experts across rounds 3 and 4, and each is a shared-surface change rather than a card-local one — which is why they were split out of #3597 instead of being worked around in the widget. `DesignSystemInlineAction` is a caption-tier tappable row for actions that belong to metadata. `_SkipAction` and `_SetupIdentityRow` were the same Material/InkWell scaffold written twice, each re-deriving radius, target, inset and state layers; both now use the component. It bakes in three things that were silently wrong or easy to get wrong at a call site: - it wraps itself in an `Align`, because a stretching parent hands children a tight width under which `MainAxisSize.min` is a no-op and the ink spans the whole column — the exact hover defect this whole redesign started from; - the inset lives inside the ink, so the rounded corners cannot cut the glyph; - semantics are excluded *below* the `InkWell` rather than above it. Excluding above — which both open-coded versions did, and which the identity row has done since it was written — drops the ink's own node along with its tap and focus actions, leaving a control that announces as a button but that assistive tech cannot activate. Caught by the component's own tests. `DesignSystemButton.alignsLabelToLeadingEdge` replaces the `Transform.translate(-inset)` #3597 had to open-code. A button's horizontal padding is internal, so a button box flush on a shared leading column puts its glyph one inset inside it: invisible beside other buttons, obvious the moment it sits in a stack of text rows. Direction-aware. `aiCard.footerWash` / `footerWashOpen` flip to a white overlay in dark at the token source. Black at 16% over the #0E1A22 card moves it [2,4,5] per channel — the band was effectively invisible — where white at 5% moves [12,11,11]. Every other wash in the group already flips direction per theme; these two were the exceptions. Light values unchanged. Regenerated via `make design_system_import`. Not done, deliberately, both recorded on the beads issue: - The panel wants `DesignSystemToggle`'s off-state thumb resolved from the surface state layers. The component already documents why it is constant: a state- or theme-varying thumb "makes OFF rows out-shine ON rows or strands a dark knob on a dark accent track". The panel could not see that rationale; the maintainer confirmed keeping it. - The panel wants the toggle to own its label. Its built-in label places the control first, forces `maxLines: 1`, and renders at `text.highEmphasis` — which would flip the footer's trailing-rail arrangement, regress the German two-line wrap the panel itself asked for, and make a metadata label louder than the state beside it.
…id-word Round 5 of the design panel came back flat — 6.58/6.50 against round 4's 6.58/6.40 — so this takes the findings that stand on their own merits and stops the loop rather than chasing a target that has stopped moving. The alert tint is now spent on the freshness glyph alone; the word stays at bodyText in both states. Tinting the word as well made the quiet settings band the most chromatic thing on the card, louder than "Confirm all" — the action that actually changes the user's task — and in dark it rendered at *lower* contrast than the plain ink it replaced. The state is still said twice, by the glyph and by the word, so colour is not carrying it alone. "Skip once" loses its underline and moves to bodyText, the register of the countdown it acts on. An action rendered fainter than the static text beside it inverts the two, and the underline gave the band a third dialect for "this is tappable" alongside the glyph and the hover fill. DesignSystemInlineAction drops the `underline` parameter outright rather than leaving it unused — its only caller was this one, and an unused knob on a shared component is how a surface acquires dialects. The attribution row becomes the same row box as the tappable row above it — matching inset, matching step8 minimum height — so the card's bottom margin no longer depends on whether the line is present. An earlier revision paid top-only padding to equalise the *declared* geometry and still shifted on screen, because the tappable row's ink box centres its glyph and contributes optical air that a bare text row does not. Space row boxes, not text. The card title falls back to one line and an ellipsis when its widest run with no break opportunity cannot fit, instead of letting the line breaker split inside the word: German at 320px and 1.3x text scale rendered "KI-Zusammenf / assung", which reads as a typo rather than as shortening. The choice is measured at the live text scaler, so the second line returns as soon as the string has room for it. No CHANGELOG entry: all four defects were introduced and fixed inside the unreleased 0.9.1070 section, so no shipped build ever showed them.
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughThe PR adds a reusable inline action component, updates AI summary card controls and identity-row spacing, improves title truncation, changes dark-mode footer tokens, and adds related tests and release documentation. ChangesAI summary card UI
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3602 +/- ##
=======================================
Coverage 99.15% 99.15%
=======================================
Files 1784 1785 +1
Lines 131153 131174 +21
=======================================
+ Hits 130041 130062 +21
Misses 1112 1112
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8ae894d73
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…unds Review findings on #3602, all three valid. The Tooltip and Semantics wrapped the Align rather than sitting inside it. The Align deliberately takes the full width a stretching parent offers — that is how it converts a tight constraint into a loose one so the ink can shrink- wrap — so anything wrapped around it inherited the full width instead of the ink's. The tooltip fired over blank footer space and the accessibility focus rectangle covered places where a tap does nothing, which is the same defect the component was extracted to prevent, surviving one layer up. Both now sit on the shrink-wrapped side. The tooltip also published its message into semantics alongside semanticsLabel, so a screen reader announced the action twice: literally so for Skip once, which passes one string as both, and near enough for the identity row, whose label already ends in "Activate to change setup". It is excluded now, leaving semanticsLabel as the single announcement the docstring always claimed it was. The existing bounds test only asserted on the InkWell, which is why this got through; it now covers the tooltip and semantics rects too. Both rewritten knowledge concepts get refreshed OKF provenance.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 234376e2e7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tags: [agents, ui, motion, accessibility] | ||
| status: stable | ||
| generated: { by: claude-code/opus-5, at: 2026-07-25T23:30:00Z } | ||
| generated: { by: claude-code/opus-5, at: 2026-07-26T14:00:00Z } |
There was a problem hiding this comment.
Fresh evidence after the earlier provenance comment: this replacement says the concept was generated at 2026-07-26T14:00:00Z, while the reviewed commit's author and committer timestamps are both 2026-07-26T13:04:50Z; the same future value appears in component-contracts.md. The immutable tree therefore claims it was generated 55 minutes after it was committed, making freshness consumers trust an impossible snapshot. Set generated.at to the actual generation time in both concepts.
AGENTS.md reference: AGENTS.md:L120-L123
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fair catch, and my error — I wrote a round 14:00:00Z rather than reading the
clock, which put the stamp 55 minutes after the commit it describes. Both
concepts now say 2026-07-26T13:20:00Z, which is behind the commit that
carries them.
make okf_check still passes: 88 concepts, 0 errors, 0 warnings.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/features/agents/ui/task_agent_automation_row_test.dart (1)
309-335: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTap the visible affordance instead of the outer slot key.
DesignSystemInlineActionshrinks its tap target to theInkWellbeneath the key, while the key wraps the full widthAlign. Tapfind.text('Skip once')or theInkWellto avoid relying on the current text position.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/features/agents/ui/task_agent_automation_row_test.dart` around lines 309 - 335, The test should tap the visible Skip once affordance rather than the outer taskAgentSkipScheduledUpdate slot key. Update the tap in the testWidgets case to target find.text('Skip once') or the underlying InkWell, while preserving the existing assertion that skips increments once.
🧹 Nitpick comments (1)
lib/features/design_system/components/buttons/design_system_button.dart (1)
201-208: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider direct component-level tests for
alignsLabelToLeadingEdge.The Transform-translate math checks out (direction-aware, cancels
sizeSpec.horizontalPadding), and hit-testing/semantics correctly follow the paint transform. However, this new public flag on a shared design-system primitive is only exercised indirectly throughtask_agent_automation_row_test.dart(icon-position assertions in stacked mode). A dedicatedDesignSystemButtontest verifying LTR vs. RTL offset direction and the no-op default would give this shared component its own regression safety net independent of any single consumer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/features/design_system/components/buttons/design_system_button.dart` around lines 201 - 208, Add direct component-level tests for DesignSystemButton’s alignsLabelToLeadingEdge behavior, covering the default false no-op and the true flag’s offset direction in both LTR and RTL layouts. Exercise the rendered button in isolation and verify the expected horizontal translation, without relying on task-agent consumer tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/features/agents/ui/task_agent_identity_region.dart`:
- Around line 172-211: Update the Tooltip in _ReportIdentityRow to set
excludeFromSemantics: true, matching the existing DesignSystemInlineAction
tooltip behavior, while preserving the current message and row rendering.
---
Outside diff comments:
In `@test/features/agents/ui/task_agent_automation_row_test.dart`:
- Around line 309-335: The test should tap the visible Skip once affordance
rather than the outer taskAgentSkipScheduledUpdate slot key. Update the tap in
the testWidgets case to target find.text('Skip once') or the underlying InkWell,
while preserving the existing assertion that skips increments once.
---
Nitpick comments:
In `@lib/features/design_system/components/buttons/design_system_button.dart`:
- Around line 201-208: Add direct component-level tests for DesignSystemButton’s
alignsLabelToLeadingEdge behavior, covering the default false no-op and the true
flag’s offset direction in both LTR and RTL layouts. Exercise the rendered
button in isolation and verify the expected horizontal translation, without
relying on task-agent consumer tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6cb70740-d0be-4b76-9e81-cac6ebe19da8
⛔ Files ignored due to path filters (1)
lib/features/design_system/theme/generated/design_tokens.g.dartis excluded by!**/generated/**
📒 Files selected for processing (14)
CHANGELOG.mdassets/design_system/tokens.jsonflatpak/com.matthiasn.lotti.metainfo.xmlknowledge/features/agents/ui-surfaces.mdknowledge/features/design_system/component-contracts.mdlib/features/agents/ui/ai_summary_card/tldr_section_part.dartlib/features/agents/ui/task_agent_automation_row.dartlib/features/agents/ui/task_agent_identity_region.dartlib/features/design_system/components/buttons/design_system_button.dartlib/features/design_system/components/buttons/design_system_inline_action.darttest/features/agents/ui/ai_summary_card/tldr_section_part_test.darttest/features/agents/ui/task_agent_automation_row_test.darttest/features/agents/ui/task_agent_identity_region_test.darttest/features/design_system/components/buttons/design_system_inline_action_test.dart
matthiasn
added a commit
that referenced
this pull request
Jul 26, 2026
Rebasing onto #3602 moved the last-commit date of `tokens.json`, the design-system theme directory and `database_config_flags.dart` — the last of those because this branch edited its comment. Two entries inherited from main were also a day ahead of the files they cite. All 238 repo-path sources match `git log -1` again.
matthiasn
added a commit
that referenced
this pull request
Jul 26, 2026
The settings band was a third of the card on a phone, and the space around the automatic-updates row was the worst of it: 33.5 logical px above the switch and 38 below, measured ink-to-ink, against declared gaps of 16 and 12. Two causes, both invisible in the source. The switch sat in a `ConstrainedBox(minWidth: step9, minHeight: step9)` whose comment claimed it "keeps a full-size interaction target for anyone who cannot land on 24px". It did not. `DesignSystemToggle` owns its own `InkWell` wrapping the 40x24 track, and that outer box is not a gesture detector — so it reserved 48px of column height while the only thing a finger could actually hit stayed the 24px track. It bought airiness and paid for nothing. And every row in the band is a touch-target box taller than its own ink, each contributing ~10px of air above and below the visible text. The declared gaps sat on top of that, so the band paid for the same space twice. So: the whole settings row is the gesture now — tapping the label toggles the setting, which is a target the width of the band rather than a 40x24 track — and it drops to the `step8` minimum every other row in the footer already uses. The row publishes `excludeFromSemantics` so it does not add a second unlabelled button node beside the switch's own. The stacked column and the gap before the identity region declare no vertical space at all; the row boxes supply the rhythm. Measured, ink-to-ink: trigger row to switch row 33.5 -> 13.5, switch row to model row 38.0 -> 22.0. Card height: 466 -> 430 on a phone, 716 -> 647 in German at 320px and 1.3x, 386 -> 366 on desktop. The split-attribution state nets to zero, because the attribution row's own step8 box costs back what the gap removal saves — the trade that keeps the card's bottom margin from moving. Also two more review findings on #3602: The report row's tooltip entered semantics. Excluding it alone would have made that row worse — its visible route sheds whole segments, so the children and the tooltip carry different strings, and excluding only the tooltip leaves the *shortened* route as the sole announcement. It now publishes one node with the untruncated attribution, the shape `DesignSystemInlineAction` uses a row above. And both concepts carried a `generated.at` of 14:00:00Z against a 13:04:50Z commit, claiming a snapshot from the future.
matthiasn
added a commit
that referenced
this pull request
Jul 26, 2026
Build only — 0.9.1070+4258 -> +4259; the version string is unchanged. The 4258 tag contains the footer redesign but not #3602, so the over-spaced settings band actually shipped. That makes this a fix to a released state rather than a defect introduced and resolved inside an unreleased version, so it carries a CHANGELOG entry and the matching metainfo line.
matthiasn
added a commit
that referenced
this pull request
Jul 26, 2026
Review findings on #3604. Making the whole settings row tappable gave it a focus stop of its own beside the switch's, so Tab landed twice on one setting and both stops toggled it. `excludeFromSemantics` suppresses the semantic node and does nothing whatever to traversal. The wrapper is pointer-only now via `canRequestFocus: false`, and a test counts focusable InkWells in the row — it reports 2 without the flag. The concept also stated the old layout contract in two places the new prose contradicted: the flow diagram's stack node still promised "a larger gap above" the schedule, and the automation section still claimed "the toggle keeps a full `spacing.step9` interaction slot around its 40x24 track" — the very claim this branch exists to delete, since that slot was inert. Both now describe the full-row target on the shared `step8` minimum. The one remaining step9 mention narrates the earlier revision as history, not as a contract. And `component-contracts.md` is dropped from this branch entirely. Its only change here was a `generated.at` rewound from 14:00:00Z to 13:56:33Z — bulk collateral from #3602 that rode along in the cherry-pick — which would have claimed an unchanged document predates its own revision. `ui-surfaces.md` keeps its bump because its prose does change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the design-system gaps behind the task-agent card footer, then applies
the round-5 panel findings that stand on their own merits.
Why this stops here
The footer has been through five rounds of design-panel review. Scores:
Round 5 moved the expert average by zero, and the expert spread collapsed to a
single point (five 6.5s and one 7) — agreement at a ceiling, not a disagreement
another round would resolve. What the panel wants next is a different, larger
footer: a new type token, four new ARB keys across 11 locales, a new last-run
timestamp field, and a countdown reformatted to "in 1m 30s" (reversing the
compact tabular countdown its own scorecard calls "excellent"). That is a
second product iteration, not polish, so it is not in this PR.
The six defects in the original brief were fixed in #3591 and #3597.
Shared components
DesignSystemInlineAction— a caption-tier tappable row for metadatacontexts.
Skip onceand the model-identity row were the sameMaterial/InkWellscaffold written twice; it bakes in three things each callsite had to get right independently:
Align, because a stretching parent hands children atight width under which
MainAxisSize.minis silently a no-op and the inkruns the width of the whole column — the original hover defect;
leading glyph;
ExcludeSemanticssits below theInkWell, never above. Both open-codedpredecessors excluded above it, which drops the ink's node along with its tap
and focus actions — a control that announces as a button but that assistive
tech cannot activate. Caught by the new component's own tests.
DesignSystemButton.alignsLabelToLeadingEdgereplaces the open-codedTransform.translate(-inset)from #3597. Direction-aware.aiCard.footerWash/footerWashOpenflipped to a white overlay in dark atthe token source. Black at 16% on the near-black card moved the surface by
[2,4,5] per channel — the band was effectively invisible.
Panel fixes
bodyTextin bothstates. Tinting it too made the settings band louder than
Confirm all, andin dark it read at lower contrast than the plain ink it replaced.
Skip onceloses its underline and moves tobodyText— the register ofthe countdown it acts on.
DesignSystemInlineActiondrops theunderlineparameter entirely rather than leaving it unused.
inset, matching
step8minimum), so the card's bottom margin no longerdepends on whether the line is present. The previous revision equalised the
declared padding and still shifted on screen: the tappable row's ink box
centres its glyph and contributes optical air a bare text row does not.
rendered
KI-Zusammenf / assung. It now measures its widest run with nobreak opportunity and falls back to one line plus an ellipsis when that run
cannot fit — truncation reads as shortening, a mid-word break reads as a
typo. The second line returns as soon as there is room.
Declined, with reasons
DesignSystemToggleowning its label. Its built-in label puts thecontrol first, forces
maxLines: 1and renders attext.highEmphasis.Adopting it would flip the footer's trailing-rail arrangement, regress the
German two-line wrap the panel itself asked for two rounds earlier, and make
a metadata label louder than the state beside it.
deliberately constant.
Known follow-up
aiCard.footerWash.darkis now byte-identical tosubtleWashStrong.dark, andthe band reads elevated in dark while it reads recessed in light. Reverting
restores the invisible band, and retuning the alpha would be an invented visual
value, so it ships as-is with the collision tracked separately. The likely real
fix is at the card-background end — lifting the body so the band can recede —
not at the band's.
Before / after
Captured from the committed opt-in screenshot matrix
(
test/features/agents/ui/ai_summary_card/screenshots_test.dart, unchanged bythis PR), so both columns come from the identical harness — "before" is
origin/main, "after" is this branch.The band is a band (dark)
Black at 16% on a near-black card moved the surface by [2,4,5] per channel.
Note also
Skip oncelosing its underline.The alert tint is the glyph's job
Out of datewas amber, which made the quiet settings band the mostchromatic thing on the card — louder than
Confirm all— at lower contrastthan plain ink.
German, 320 px, 1.3× text scale
KI-Zusammenfassunghas one break opportunity, and the token after it stilloverflows, so the line breaker split inside the word:
KI-Zusammenf / assungreads as a typo. It now truncates, which reads as shortening.
Attribution no longer changes the card's bottom margin
The
This report …line sat flush against the card edge while its absenceleft the tappable row's ink box supplying optical air.
Verification
dart analyze lib test— no issues.flutter test test/features/agents/ test/features/design_system/—6165 passed, 2 skipped, 0 failed.
manual/scheduled/running/proposals/split/hover, plus German at 1.3x; the
title truncation, the missing underline and the glyph-only tint are all
confirmed in pixels.
No CHANGELOG entry for the four panel fixes: each defect was introduced and
fixed inside the unreleased 0.9.1070 section, so no shipped build showed them.
The dark-mode band entry from the first commit remains, since that one was
released.
Summary by CodeRabbit
New Features
Bug Fixes