fix: task-relationship-links — production defects, accessibility, and the finish pass#3575
Conversation
Three defects, found by screenshotting states no review round had ever captured. All three shipped. The edit-relationship modal was unusable. Its Save footer is a sticky overlay, not a sibling, so without reserving its height the relation dropdown rendered underneath it: the modal showed a title, a Save bar, and nothing to edit. The tests passed because they drive the dropdown through its callback — this repo's pattern for controls that are awkward to tap — and a callback cannot notice that the widget is covered. There is now a geometry assertion that the control sits above the footer, verified to fail when the fix is reverted. The manage-mode exit was a bare Material TextButton, rendering as an unlabelled lavender slab — the one control on the surface outside the token system, and it looked it. It is a design-system tertiary button now. The single-select dropdown check was painted in the checkbox's on-fill ink, which is meant to sit inside a filled box. Standing on the panel it was (13,13,13) on (24,24,24) — darker than its own background, so the mark added last round was invisible in practice. It has its own token now, which fixes current-value legibility for every single-select dropdown in the app. Also from the same review: section headers share the list item's leading gap and glyph size so header labels land on the row-title rail; the first section loses the top pad that stacked with the card header into the largest gap on the card; single-select picker lists get a closing inset instead of sitting flush to the sheet edge; the blocked-by chip drops to outline so the red status pill is the header's only alarm, with its chevron in the chip's own accent; and the trailing ASCII "..." in four strings becomes a real ellipsis, matching the entry-point wording it sits beside.
…ady the rails The edit modal said "This task… / Is blocked by" and never named the other task — a sentence missing its object, and the one remaining comprehension gap three reviewers agreed on. Both call sites now pass the counterpart's title. The relation dropdown was set in bodyLarge, so a relation phrase outranked the task titles it describes and the card header above it. It drops to bodyMedium, its menu rows lose ~20% of their pitch, and the panel's ceiling is now a whole number of rows, so the last visible option is no longer sliced through its text. The trailing rail on a linked-task row was a different width in manage mode than in browse mode, so toggling the mode re-wrapped every title on the card. Both states now occupy the same box. Colour and rank, all one-line token changes: the blocked-by chip keeps amber for its border and glyph but takes an ordinary label, so an explanatory chip stops out-shouting the status pill it explains; section headers move to the overline style at high emphasis while row status drops to low, breaking a tie where relationship semantics ranked equal to the word "Open"; the overflow glyph stops being the heaviest mark in the card header. The unlink confirmation was a raw Material AlertDialog inside an otherwise tokenised feature — the last surface that dropped out of the design system. It uses the shared confirmation modal now. Also: icon sizes and one spacer literal onto tokens, and the empty-state action gets the same trailing chevron every other navigable row has.
Round 9 caught two round-8 fixes that never landed, and four reviewers measured them independently rather than taking the changelog at its word: - The dropdown panel ceiling was derived from bodySmall while the rows render bodyMedium, so the panel stood 6.4 rows tall and sliced the seventh option through its glyphs. - Browse mode reserved one action box in the trailing rail while manage mode occupied two, so toggling the mode still re-wrapped every title. Both now carry regression tests that measure the laid-out result — panel height against row pitch, and the title's box across both modes — and each was verified to fail when its fix is reverted. The earlier tests passed either way, which is exactly how the defects shipped. Also in this round: - Section eyebrows drop to medium emphasis and row status to low, so title, relationship and status are three distinct levels rather than three roles tied at #FFFFFF. - The blocked-by chip is down to two amber marks; its chevron is neutral, since "go here" is not part of the blocked semantic. - The chip says "Blocked by N tasks", matching the section it navigates to and the option that created the link. "Waiting on" solved two red pills shouting the same word — a visual problem, since solved visually by the outline treatment. - "Create new linked task" moves off a raw Material AlertDialog onto the shared modal, the last surface in the feature outside the design system. - The edit modal's counterpart line loses the leading ellipsis that read as truncation, becoming a caption label over the title at high emphasis. - Manage mode keeps the row status: curating links is when knowing a blocker is already done matters most.
Round 10's hierarchy review found the emphasis ladder only held on desktop. The wide layout styles the status label directly, but the narrow one drops it into the list item's subtitle slot, which paints medium emphasis — the exact tie with the section eyebrow the change was meant to break. Overridden via spans so both layouts rank the same three roles. Two of the geometry tests were measuring the wrong thing: WidgetTestBench hands its child tight constraints, so a SizedBox width is silently ignored and every case rendered at the full 800pt surface. The rail test still caught its regression by luck of the layout; the emphasis test would have passed against either implementation. Both now constrain through Align and were checked to fail when their fix is reverted. Also: - Picker result rows carry a trailing link glyph. They are one tap from the card's rows and read identically — same glyph, title and status — while tapping here creates a link and tapping there opens the task. Nothing on the row said which. - Manage mode ranks its verb above its escape hatch: edit takes medium emphasis and unlink drops to low. The confirmation modal is what makes unlinking safe; painting it as the brightest mark on the row only drew the eye toward the destructive action. - Section eyebrows clear the following row's own top padding and line leading, so proximity binds each label to its rows instead of stranding it between two groups.
Opening a blocker from the multi-blocker sheet navigated while the sheet was still up, so on a desktop layout the pane changed behind the barrier and the user was left looking at an unchanged sheet. The only tap in the feature that produced no visible result. The sheet now records the pick, closes, and navigates from the caller's context; LinkedTaskRow takes an onOpen override for surfaces that must dismiss themselves first. The trailing rail had width parity but not height: the action pair is step8 tall while the browse chevron reserved no height at all, so desktop rows grew 20pt and the card jumped taller whenever Manage turned on. Both axes are reserved now, the actions are pinned to the box rather than merely bounded by it, and a height regression test covers the axis the width test could not see — verified to fail on revert. Also: the two modal bodies that reserve the glass footer no longer add a spacing step on top of it, which had left the edit modal bottom-heavy against its own top inset. Not done, deliberately. Constraining the header's overflow button to the same rail as the row actions is a real alignment win, but Material derives the popup menu's width from the button's box, so the menu's own items overflow. Not worth trading a working menu for 8pt of alignment.
The search field painted Material's default lilac caret and selection — the only colour on any of these surfaces that belonged to no token. Set from the interactive token, which fixes it everywhere the field is used. Amber now marks only the ⊘ glyph, never the eyebrow label. The label's orange sat one hue away from an On Hold row's own orange: close enough to read as the same thing while meaning something else. The glyph carries the semantic in a shape no status glyph uses. Also: - One colour for the link glyph. Emptying the card recoloured its own icon, because the empty state and the header painted the same Icons.add_link from two different tokens. - Three hand-rolled Material Dividers become DesignSystemDivider, the picker's CircularProgressIndicator becomes DesignSystemSpinner, and the last raw `size: 16` moves onto the spacing scale. - Modal titles drop the ellipsis their opening control keeps. "…" promises further UI; the modal is that UI, so the title should not promise it again. The link modal's top third had stacked three ellipses where only one — the "This task…" sentence stem — was doing real work. - The link modal is bounded at the picker's own measure, so opening the relation list takes space from the results rather than pushing the sheet to the full height of the screen. It does not stop the sheet resizing outright: the shared dropdown expands inline rather than over its host, and changing that is a change to every dropdown in the app.
…dings Full-text matches were intersected against the 200 open tasks the picker had prefetched, so on a larger backlog a task you searched for by name came back as "No tasks found" — the picker stating confidently that it isn't there. Matches outside the window are now fetched by id and filtered to open tasks. Two tests cover it, and the first was verified to fail against the old code. Also from round 11: - The card's add-link action is a labelled button. As a bare glyph it was the card's only creative action and its least legible control, and it gave a low-vision reader nothing to read once the card had any links. - Save in the edit sheet is inert until the relation actually changes. An identity round-trip bumped the vector clock and enqueued a sync message for an edit nobody made. The tests that asserted that write now assert the direction mapping in both directions instead, which is what they were really for. - The blocker picker carries the same amber ⊘ as the section and header chip that read the relationship back. It was the only surface in the feature stating its meaning with a sentence and nothing else, on the highest consequence tap in the set. - The picker's link glyph is no longer dimmer than the status text beside it; it is the only thing distinguishing a row that commits from one that navigates. - French and Romanian task statuses were all caps while every other language was sentence case, which shouted the quietest rank in the row. - The edit sheet's two field labels share one ink, its read-only value drops below the editable one, and the link modal's inter-field gap matches the gap around it.
Round 11 made full-text matches from outside the prefetch window visible, but the pick handler still resolved ids against the prefetched list alone, so tapping one of those rows threw StateError. The test I wrote for that fix asserted the row rendered and never tapped it — which is exactly how a fix for "the picker denies a task that exists" shipped as "the picker crashes on that task instead". Rows and picks now read one candidate pool, and the test taps the row and asserts the link is written. Verified to fail on revert. Also from round 12: - The blocker picker's title is back on the shared modal chrome. Putting the ⊘ in the title bar made it compete for a width the close button dictates, clipping "What's blocking this?" mid-word on phone in English, before any longer locale. The mark moves into the body as a section header, where it also names the relation the pick will write. - The disclosure chevron leads the title it discloses. Sitting against the Link button it read as that button's caret, so a tap aimed at a relation menu collapsed the card — and it disappeared in manage mode while the collapse gesture stayed live. - DesignSystemListItem gains a subtitle-emphasis contract, plumbed through the selection row and picker item. That removes this feature's only raw TextStyle and settles the status metadata at one ink whether it is read on the card or in the picker one tap later. - The empty card's link glyph takes the interactive accent the populated card's Link button carries; emphasis should be highest where the user has nothing yet. The comment claiming they already matched was wrong. - Two measured proximity ties: the tight-top eyebrow gap and the edit sheet's group gap.
`getTasks` short-circuits an empty `categoryIds` to `WHERE 1 = 0`, and the picker passed `categoryIds: []` meaning "no filter". So the browse list returned nothing at all in production, and every test passed because they mock `getTasks` and never exercise the query builder. JournalQueryRunner already expands the same way for the same reason; the picker now does too, with a test that pins the contract and fails against the old call. Also from round 13: - A task that has links no longer shows "you have no links" on cold open. The provider resolves two DB round trips, and treating "no value yet" as "no links" put a definitive empty CTA in front of every populated card for the length of that load. - Finished tasks can be linked. "Follows up on", "Duplicates", "Fixes" and "Supersedes" all routinely reference work that is already done, and the modal reported "No tasks found" for a task the user can see elsewhere in the app. The blocker picker keeps the open-only filter, since a finished task cannot block anything. - Undo is awaited and reports failure, matching the unlink path it mirrors. Fire-and-forget made a failed undo indistinguishable from a successful one, on the one control whose whole job is taking something back. Its window is also longer than the 4s default: it is the only recovery from a link that commits on a single tap, and it arrives at the bottom of the window while the eye is still on the modal that was dismissed. - Manage-mode actions are 48pt, the Material minimum and above Apple's 44. They were 40, in an adjacent pair, above a row that is itself tappable — and a comment claimed 40 was the design system's minimum interactive target, which it is not. Pinned by a test.
A plain link reads the same from either end, but `inverse` still took part in DirectedRelation's identity, so an existing incoming plain link never matched the one the picker offers. The picker re-offered a task already listed on the card, and taking it wrote a second identical row, inflated the count badge, and cost two confirmations to undo one relationship. `inverse` is now normalised away on symmetric relations, with tests for both halves: the plain link is excluded from either side, while "blocks" and "is blocked by" stay independently offerable, since a pair may legitimately hold both. Failure messages also stopped blaming a cause that cannot apply. Only a blocking edge can fail the cycle guard, but every rejected link — "Relates to", "Duplicates", anything — reported "This would create a blocking cycle" and offered a remedy that would not help. Also from round 14: - Failures no longer wear the success colour. A rejected cycle, a failed unlink, a failed retype and a failed undo all arrived on the same Material slab as the "link created" confirmation, tinted by whichever scheme the user picked. One shared helper puts them on the alert colour; it takes resolved tokens rather than a context because every caller reports after awaiting the write that failed. - Manage mode keeps an add path. Its header slot is taken by Done, which left no way to add a link while curating them — the state where wanting one is most likely. - The blocker picker's relation statement sits on the picker's own rail instead of 25pt left of the rows it labels, and has room above the search field. - The empty card's explanation is quieter than the action it explains and survives three lines, so the longer locales stop ellipsizing away the examples that teach what "link" means here.
Both the confirmation and the four failure messages were hand-built SnackBars. They now go through DesignSystemToast: success tone with the undo action and a countdown strip that makes the remaining window visible, error tone for failures. That also retires a bug I introduced last round — a hand-picked ink on the alert fill rendered near-black on dark red in light theme, on the one message the flow shows when it refuses. The component owns its own on-colour. A rejected retype no longer says "please try again" for a cause that makes retrying pointless. The cycle guard is the only thing that rejects a retype and it can only reject a blocks edge, so that case now names the cycle. Same defect class as the create path, closed on the last surface holding it. Modal titles fit one line at phone width in every shipped locale. German ran 33 characters against a 230pt title box — half again the English length — and no screenshot review sees a non-English string, so this was invisible for fifteen rounds. A new test measures all four titles in all six locales against the real font and fails on a second line; it caught one more overflow after the first pass of shortening. Deliberately not done: halving the browse-mode trailing reserve. It would give back 48pt of measure on phone rows, but it re-creates the width mismatch between browse and manage mode that rounds 10, 12 and 13 each independently flagged, and that a regression test now pins.
At text scale 1.3 through 2.0 the header Row overflowed by up to 158pt on a 390pt phone, clipping the Link action and dropping the overflow menu entirely — which in manage mode is one of only two ways out. Large text is a supported configuration, not an edge case. The card's own name is now the part that gives: flexible and ellipsized, so the controls survive. The test that should have caught this was measuring an 800pt card. The bench hands its child tight constraints, so the width it was given was ignored — the same harness flaw that made two geometry tests no-ops in round 10. The helper now constrains properly, and the scale cases were checked failing on revert. Also: - The undo toast dismisses itself. It removed the link but left the confirmation, its live Undo control and a draining countdown on screen for another ten seconds, asserting a link that no longer existed and offering to remove it again. - The linked task's name moved out of the toast's one-line title into its description slot. Relation plus a real title ran past one line, so the message ellipsized away the one fact it exists to record — on a flow that commits without confirming. - Modal titles are bounded to one ellipsized line in the shared component. A title needing a second line silently reflowed a fixed-height bar; now it degrades predictably, and the route semantics still announce it in full. - The title-fit guard was measuring the wrong thing: it omitted the shipped letterSpacing, under-reporting by ~3pt on ~5pt of headroom. Corrected, and it promptly failed until the remaining long titles were shortened.
Last round's accessibility fix made the header title Flexible while leaving the Spacer beside it. Both carry flex 1, so they split the free space evenly: the title gave way to "Linked T…" at normal text size while most of the row sat empty. Four reviewers found it independently. The title and its badge now share one Expanded and the Spacer is gone, so the group takes whatever the trailing controls do not need and gives way only when there is genuinely nothing left. The guard added last round asserted the header does not overflow and nothing about whether the title survived, which is how this shipped under a green suite. It now also asserts the title is not truncated at default text size, in both modes — and it needed two corrections before it meant anything: the suite renders with a fallback font far wider than Inter, and it measured the card at the full viewport width rather than the ~357pt it actually gets inside the page's padding. Verified failing on revert with the real numbers. Also: the unlink action is `Icons.link_off`. It was `Icons.close_rounded` — which is StatusGlyph's own icon for Rejected, so a rejected row in manage mode drew the same mark twice with two different meanings. The edit action already avoids exactly this collision for Groomed.
Last round's fix was verified in English and shipped truncated in German and Spanish, whose card title and action label are both markedly longer. The action now gives up its word before the card gives up its name: the header measures both strings and falls back to an icon-only link button, still named by tooltip and semantics, where the worded one will not fit. Which locales collide is a property of the strings, so it is measured rather than guessed at a breakpoint. The guard loops over all six shipped locales now. It catches exactly de and es when the worded button is forced — the two locales the review named — and this is the third guard in this feature that only meant something after being made to fail first. Also: - The empty card words both creative actions. Creating a linked task was reachable only through an unlabelled overflow glyph holding a single item, on the one screen whose job is to teach what the card is for. - The unlink confirmation names the task. The rows it is reached from carry two faint glyphs each, so "this task" could not say which link was hit — on the feature's only irreversible action.
Its chevron sat bare, on a 40pt target this feature elsewhere calls under-minimum, and the card's trailing rail jumped 18pt the instant it gained its first link. Both rows now use the same reserved rail every populated row uses, extracted so the two cannot drift apart again. Spacing called this the one thing it would hold a ship for. The empty card's action titles also take two lines, like the task titles beside them elsewhere on the card. A one-line cap ate the verb in verb-final German — on labels I promoted out of the overflow menu last round precisely so they could be read. Also: - The blocked-by accent drops once every blocker is closed. A closed blocker releases the dependent, so amber there asserted something the header had already retracted, on the feature's only semantic colour. - The header's fit allowance comes from DesignSystemButton's own size spec rather than a round number roughly double the button's real chrome — a guess that was taking the label away from locales with room for it. - The icon-only fallback is a DesignSystemButton rather than a raw Material IconButton, so a user's locale no longer decides whether they get a design-system control. - The empty card has no overflow menu: its only item is now a worded row in the body, and an unlabelled glyph was the loudest mark on the screen that has to teach the feature.
The change was written but never saved: the script that applied it hit a failing assertion on a later edit and threw before writing the file, and I re-ran only the other half. The commit message and the tracker note both said it was done. It was not — the 96pt stand-in survived, taking the Link label away from Romanian and Czech, which have room for it. The allowance now derives from DesignSystemButtonSize.small's own spec (2*step3 + lineHeight.subtitle2 + step2 = 40), so ro and cs keep their word while de and es stay correctly icon-only. The locale guard now asserts both directions — that the worded label is present where it fits and absent where it does not. Asserting only that the title survives is what let an allowance 2.4x too large pass through two rounds and one "fixed" claim. Verified failing on revert, naming exactly ro and cs.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 9 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (32)
📝 WalkthroughWalkthroughThe PR refines linked-task relationship flows, search and picker behavior, responsive linked-task layouts, shared modal/toast feedback, design-system styling, localization, documentation, and regression coverage. ChangesLinked Tasks UX
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/l10n/app_localizations_cs.dart (1)
10282-10288: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a complete Czech phrase for the plural blocker tooltip.
Klepni pro $count blokujícíchis incomplete and unnatural for counts greater than one. Update the ARB source to something likeKlepni pro zobrazení $count blokujících úkolů, then regenerate this file rather than editing generated Dart directly.🤖 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/l10n/app_localizations_cs.dart` around lines 10282 - 10288, Update the Czech ARB source for taskBlockedByChipTooltip so the plural message is a complete phrase, such as prompting users to view the specified number of blocking tasks; then regenerate app_localizations_cs.dart from the ARB source instead of editing the generated Dart directly.Source: Coding guidelines
🧹 Nitpick comments (4)
lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPulling
isClosedTaskfromdaily_os_next/agents/servicecrosses feature boundaries.A task-status predicate consumed by the tasks UI belongs in the tasks feature (or a shared util), not in a
daily_os_nextagent service. Consider relocating/re-exporting it so the linked-tasks card does not depend on an unrelated feature's service layer.Also applies to: 99-101
🤖 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/tasks/ui/linked_tasks/task_relationship_sections.dart` at line 4, Move or re-export the isClosedTask predicate from daily_os_next agent service code into the tasks feature or an appropriate shared utility, then update the linked-tasks UI imports and usages to reference that location. Remove the cross-feature dependency while preserving the existing task-status behavior.test/features/tasks/ui/linked_tasks/modal_title_fit_test.dart (2)
36-45: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBrittle production-source string search instead of a widget assertion.
Slicing
modal_utils.dart's raw source between two method-name substrings is fragile: if_tokensis reordered/renamed relative to_modalTitle,indexOfreturns-1andsubstring(-1, x)/substring(x, -1)throws, failing the test with a confusingRangeErrorunrelated to the actual regression being guarded (a two-line modal title). Prefer pumping the modal (or the shared title widget) and assertingText.maxLines/Text.overflowdirectly on the rendered widget.🤖 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/tasks/ui/linked_tasks/modal_title_fit_test.dart` around lines 36 - 45, Replace the raw source slicing in the modal title test with a rendered-widget assertion: pump the modal or shared title widget, locate its Text widget, and assert maxLines is 1 and overflow is TextOverflow.ellipsis. Remove the File, substring, and source-search logic while preserving coverage of the two-line modal title behavior.
62-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHardcoded style duplicates (doesn't verify) the production style.
fontFamily/fontSize/fontWeight/letterSpacinghere are copied constants, not read frommodal_utils.dart. If the design system changes the modal title style, this test keeps measuring the old values and can pass (or fail) independently of what actually ships, defeating the purpose of the regression guard described in the file's own doc comment.🤖 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/tasks/ui/linked_tasks/modal_title_fit_test.dart` around lines 62 - 78, The TextPainter setup in the linked-task modal title fit test hardcodes production typography values, so it can diverge from the shipped style. Reuse the modal title style defined in modal_utils.dart for the TextSpan instead of duplicating fontFamily, fontSize, fontWeight, and letterSpacing constants, while preserving the existing text scaling, line limit, and layout behavior.test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart (1)
651-663: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: extract a shared helper for the "open menu → create new linked task → pick relation" sequence.
The overflow-menu-open and "Create new linked task…" tap sequence is duplicated identically across three tests. Consider parameterizing
tapCreateNewLinkedTask(or a sibling helper) with an optional relation label to pick before confirming, removing the repeated boilerplate.♻️ Sketch of a shared helper
- Future<void> tapCreateNewLinkedTask(WidgetTester tester) async { + Future<void> tapCreateNewLinkedTask( + WidgetTester tester, { + String? pickRelationLabel, + }) async { await tester.tap(find.byIcon(Icons.more_vert)); await tester.pump(); await tester.pump(const Duration(milliseconds: 300)); await tester.tap(find.text('Create new linked task…')); await tester.pump(); await tester.pump(const Duration(milliseconds: 300)); + if (pickRelationLabel != null) { + await pickRelation(tester, pickRelationLabel); + } await tester.tap(find.text('Create')); await tester.pump(); await tester.pump(const Duration(milliseconds: 300)); }Also applies to: 709-720, 766-777
🤖 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/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart` around lines 651 - 663, Optionally consolidate the duplicated overflow-menu, “Create new linked task…”, relation selection, and confirmation sequence used by the three tests into a shared helper near tapCreateNewLinkedTask. Parameterize it with an optional relation label so callers can select the relation before tapping Create, while preserving the existing timing and behavior for callers without a relation.
🤖 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/design_system/components/dropdowns/design_system_dropdown.dart`:
- Around line 318-324: Update the menu size calculation around menuItemMinHeight
and panelMaxHeight to accept and use the effective TextScaler, scaling the
tokens.typography.lineHeight.bodyMedium value before adding vertical spacing.
Derive both the rendered row height and seven-row panel ceiling from that scaled
line height so enlarged text scales without clipping the final option.
In `@lib/features/tasks/ui/linked_tasks/link_created_feedback.dart`:
- Around line 64-78: Update the undo handler in link_created_feedback.dart to
capture removeTypedLink’s delete count and show unlinkTaskFailedMessage when the
result is <= 0, while retaining thrown-error handling. Propagate the same
success indicator through onUnlink in linked_task_row.dart so _confirmUnlink can
report non-throwing unlink failures instead of treating every completed call as
success.
In `@lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart`:
- Around line 643-650: Update _measureText to store painter.width in a local
variable after layout, dispose the TextPainter, then return the stored width.
In `@lib/l10n/app_cs.arb`:
- Line 4157: Update the Czech taskBlockedByChipTooltip plural translation so the
other branch includes a natural action and the noun “úkolů,” such as prompting
users to view the count of blocking tasks; preserve the singular branch and
plural structure.
In `@lib/l10n/app_localizations_fr.dart`:
- Around line 5668-5690: Update the habitStreakDaysSemantic translation in
app_fr.arb so both plural branches render d'affilée with a single apostrophe,
then run make l10n to regenerate the localization output; do not edit the
generated habitStreakDaysSemantic entry in app_localizations_fr.dart directly.
In `@lib/l10n/app_localizations_it.dart`:
- Around line 10938-10941: Add the missing unlinkTaskConfirmNamed translation
key to lib/l10n/app_it.arb with the correct Italian localized text, then run
make l10n to regenerate app_localizations_it.dart. Do not edit the generated
unlinkTaskConfirmNamed method directly.
In `@lib/l10n/app_localizations_pt.dart`:
- Around line 4346-4348: Update dailyOsNextTimeSpentSummary and its app_pt.arb
source entry to use singular wording for one completed item and plural wording
for completedCount greater than one, then regenerate the localization output so
the generated Portuguese file reflects the ARB plural forms.
In `@lib/l10n/app_localizations_ro.dart`:
- Line 4457: Update the corresponding Romanian entries in lib/l10n/app_ro.arb to
use formal imperatives: “Faceți acum”, “Asociați”, “Gestionați legăturile…”,
“distrugeți...creați”, and “Atingeți”. Regenerate the generated localization
output with make l10n, covering all listed sites in
lib/l10n/app_localizations_ro.dart.
- Around line 6724-6725: Update the Romanian localization value for
linkCreateFailedMessage to use the grammatically correct retry wording, “Vă
rugăm să încercați din nou.” Modify the corresponding ARB source and regenerate
the localization output so app_localizations_ro.dart reflects the change.
In `@lib/l10n/app_ro.arb`:
- Around line 4142-4150: Update the plural branch of taskBlockedByChipTooltip to
use the established formal imperative, replacing the informal “Atinge” wording
while preserving the count placeholder and all other pluralization branches.
In `@test/features/tasks/ui/linked_tasks/linked_tasks_widget_test.dart`:
- Around line 744-769: Update the text-scale survival tests in the nested
scale/manageMode loop to render the widget at the actual approximately 357pt
card width rather than the full 390pt viewport. Keep the existing scale values,
manage-mode coverage, exception assertion, and more_vert visibility assertion
unchanged so the tests exercise the intended constrained-header regression.
---
Outside diff comments:
In `@lib/l10n/app_localizations_cs.dart`:
- Around line 10282-10288: Update the Czech ARB source for
taskBlockedByChipTooltip so the plural message is a complete phrase, such as
prompting users to view the specified number of blocking tasks; then regenerate
app_localizations_cs.dart from the ARB source instead of editing the generated
Dart directly.
---
Nitpick comments:
In `@lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart`:
- Line 4: Move or re-export the isClosedTask predicate from daily_os_next agent
service code into the tasks feature or an appropriate shared utility, then
update the linked-tasks UI imports and usages to reference that location. Remove
the cross-feature dependency while preserving the existing task-status behavior.
In `@test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart`:
- Around line 651-663: Optionally consolidate the duplicated overflow-menu,
“Create new linked task…”, relation selection, and confirmation sequence used by
the three tests into a shared helper near tapCreateNewLinkedTask. Parameterize
it with an optional relation label so callers can select the relation before
tapping Create, while preserving the existing timing and behavior for callers
without a relation.
In `@test/features/tasks/ui/linked_tasks/modal_title_fit_test.dart`:
- Around line 36-45: Replace the raw source slicing in the modal title test with
a rendered-widget assertion: pump the modal or shared title widget, locate its
Text widget, and assert maxLines is 1 and overflow is TextOverflow.ellipsis.
Remove the File, substring, and source-search logic while preserving coverage of
the two-line modal title behavior.
- Around line 62-78: The TextPainter setup in the linked-task modal title fit
test hardcodes production typography values, so it can diverge from the shipped
style. Reuse the modal title style defined in modal_utils.dart for the TextSpan
instead of duplicating fontFamily, fontSize, fontWeight, and letterSpacing
constants, while preserving the existing text scaling, line limit, and layout
behavior.
🪄 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: 56e27f6e-cf9d-4c83-9183-dba94e732fa8
📒 Files selected for processing (53)
CHANGELOG.mdflatpak/com.matthiasn.lotti.metainfo.xmllib/features/design_system/components/dropdowns/design_system_dropdown.dartlib/features/design_system/components/dropdowns/design_system_dropdown_panel.dartlib/features/design_system/components/lists/design_system_list_item.dartlib/features/design_system/components/search/design_system_search.dartlib/features/design_system/components/selection/design_system_selection_row.dartlib/features/tasks/README.mdlib/features/tasks/ui/header/desktop_task_header_connector.dartlib/features/tasks/ui/linked_tasks/blocking_task_picker_modal.dartlib/features/tasks/ui/linked_tasks/edit_link_type_modal.dartlib/features/tasks/ui/linked_tasks/link_created_feedback.dartlib/features/tasks/ui/linked_tasks/link_task_modal.dartlib/features/tasks/ui/linked_tasks/linked_task_row.dartlib/features/tasks/ui/linked_tasks/linked_tasks_widget.dartlib/features/tasks/ui/linked_tasks/relationship_type_selector.dartlib/features/tasks/ui/linked_tasks/task_relationship_sections.dartlib/features/tasks/ui/linked_tasks/task_search_picker_body.dartlib/l10n/app_cs.arblib/l10n/app_da.arblib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_it.arblib/l10n/app_localizations.dartlib/l10n/app_localizations_cs.dartlib/l10n/app_localizations_da.dartlib/l10n/app_localizations_de.dartlib/l10n/app_localizations_en.dartlib/l10n/app_localizations_es.dartlib/l10n/app_localizations_fr.dartlib/l10n/app_localizations_it.dartlib/l10n/app_localizations_nl.dartlib/l10n/app_localizations_pt.dartlib/l10n/app_localizations_ro.dartlib/l10n/app_localizations_sv.dartlib/l10n/app_nl.arblib/l10n/app_pt.arblib/l10n/app_ro.arblib/l10n/app_sv.arblib/widgets/modal/modal_utils.dartlib/widgets/picker/entity_picker_sheet.darttest/features/design_system/components/dropdowns/design_system_dropdown_test.darttest/features/tasks/ui/header/desktop_task_header_connector_test.darttest/features/tasks/ui/linked_tasks/blocking_task_picker_modal_test.darttest/features/tasks/ui/linked_tasks/edit_link_type_modal_test.darttest/features/tasks/ui/linked_tasks/link_task_modal_test.darttest/features/tasks/ui/linked_tasks/linked_task_row_test.darttest/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.darttest/features/tasks/ui/linked_tasks/linked_tasks_widget_test.darttest/features/tasks/ui/linked_tasks/modal_title_fit_test.darttest/features/tasks/ui/linked_tasks/task_relationship_sections_test.dart
This branch loads real fonts in two of its own tests, and CI runs the suite in one isolate, so those metrics become the suite's. That surfaced a fragile assertion in an unrelated test: it measured the width of the centred *label* "Start using AI" and required it exceed 120pt, as a proxy for "the button fills the row". Under the fallback font that label lays out at 231pt; under real Inter it is 111pt, and the test failed on shard 5. The assertion now measures the button against the surface it sits in, which is what the test's own comment says it means to check and what no font can move.
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (97.78%) is below the target coverage (99.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3575 +/- ##
==========================================
- Coverage 99.14% 99.14% -0.01%
==========================================
Files 1782 1782
Lines 130431 130617 +186
==========================================
+ Hits 129319 129496 +177
- Misses 1112 1121 +9
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:
|
…aling
- `_measureText` disposed the TextPainter it allocates. It runs on every
header build and the painter holds native paragraph resources.
- Unlink and undo report a delete that removed nothing. `removeTypedLink`
returns a row count and throws nothing when it matches no row, so a no-op
removal was indistinguishable from a successful one — the row simply stayed
put. `onUnlink` now carries the count through to the confirmation handler.
- The dropdown's panel ceiling scales with the text size its rows render at.
Row height grows with the user's setting while the surrounding padding does
not, so an unscaled ceiling stopped mid-row — the very defect the
calculation exists to prevent, one accessibility setting out. Covered at
1.6x, verified failing on revert at 336pt against 62pt rows.
- The header text-scale tests measure the card's real ~357pt width, matching
every other assertion in that file. At the full viewport they were handed
33pt the shipped card never has, which is enough headroom to keep the
overflow menu visible when the real card would drop it.
Localization:
- Romanian keeps the formal register throughout, and the retry sentence gets
its conjunction ("Vă rugăm să încercați din nou").
- The Czech plural tooltip reads as a sentence rather than a fragment.
- Removed `editLinkTypeCounterpart`, a dead key this branch had deleted and a
merge resurrected.
- ARB files keep main's existing key order. An earlier union merge re-sorted
them, which produced ~900 lines of pure reordering across eleven files and
buried the twenty lines that actually changed.
Finishes the task-relationship-links UI from #3556 / #3563 (ADR 0042/0043).
Twenty rounds of the multi-agent design panel, ending with both panels at
8.0/10 and every individual reviewer at 8 — six expert lenses and five user
personas, the personas with zero blockers between them.
The interesting part is not the polish. Once reviewers started reading the code
behind the controls rather than only looking at pixels, they found production
defects that no screenshot could show. Those are the bulk of this PR.
Real defects fixed
The pickers listed nothing at all in production.
getTasksshort-circuitsan empty
categoryIdstoWHERE 1 = 0, and the picker passed[]meaning"no filter". Browsing returned nothing; only a complete-word FTS5 match found
anything. Every test passed, because they mock
getTasksand never reach thequery builder.
JournalQueryRunneralready expands empty → all categories +''for exactly this reason.Search could deny a task that exists, then crash on the one it found.
FTS5 hits outside the 200-row prefetch window were filtered against only that
window, so a task past the 200th row came back as "No tasks found". Fixing that
made those rows visible while
onPickstill resolved ids against the prefetchedlist alone — tapping one threw
StateError. Rows and picks now read onecandidate pool.
A task already linked could be linked again. A plain link reads the same
from either end, but
inversetook part inDirectedRelation's identity, so anexisting incoming plain link never matched the one the picker offers. Taking
it wrote a second identical row, inflated the count badge, and cost two
confirmations to undo one relationship. The suite never constructed an
ExistingRelationwithinverse: true, which is how it survived fourteenrounds.
A tap that did nothing. Opening a blocker from the multi-blocker sheet
navigated while the sheet was still up, so on desktop the pane changed behind
the barrier.
Finished tasks were unlinkable. "Follows up on", "Duplicates", "Fixes" and
"Supersedes" all routinely reference completed work, but closed tasks were
excluded and the modal reported "No tasks found" for a task visible everywhere
else. Blockers still list open tasks only.
Every failed link blamed a blocking cycle, including relationships that
cannot create one — naming a cause that could not apply and a remedy that would
not help.
The card header overflowed at accessibility text sizes by up to 158pt on a
390pt phone, clipping the action and dropping the overflow menu — which in
manage mode is one of only two ways out. And it truncated its own name to
"Linked T…" at default size in German and Spanish.
A task with links showed "you have no links" on cold open, because the
widget read
.value ?? emptywith no loading branch across two DB round trips.Undo was fire-and-forget — a failed undo was indistinguishable from a
successful one, on the control whose entire job is taking something back.
What it looks like
The card
Relationships group under the exact phrase the picker offered. "Is blocked by"
leads and is the only accented section — and the amber drops once every blocker
is closed, since a closed blocker releases the dependent. Three ink levels:
task titles brightest, section eyebrows medium, status quietest.
Empty and manage states
Both creative actions are worded on the empty card. Manage mode says it is on,
keeps each row's status, and reserves the same rail as browse mode so toggling
never reflows a title.
Picking a task and a relationship
Type and direction are one choice: eleven directed phrases completing "This
task…". Result rows carry a link glyph, because they read identically to the
card rows one tap away while committing rather than navigating.
Editing and blocking
Save stays inert until the relation actually changes — an identity round-trip
bumped the vector clock and enqueued a sync message for an edit nobody made.
Header chip
Count-only and outline, with amber on the border and the ⊘ only. It says
"Blocked by N tasks" — the same words as the section it opens and the option
that created the link.
Notes for review
became real after being made to fail first: one omitted the shipped
letterSpacing, one measured an 800pt card the bench had stretched, one useda fallback font far wider than Inter, one tested English only. Every geometry
and locale assertion here has been checked against its own revert.
alert.warningcontrast (2.15–2.38:1, against a passing 3.29:1 step alreadyin the ramp) went unseen for twenty rounds. Filed as a design-system bug; the
widgets bind correctly and are not patched around it.
work by two or more rounds: the inline-expanding dropdown panel and its host
resize, dropdown elevation/scrollbar/TextScaler, the picker-vs-card row rail,
DsPillheight, andDesignSystemListItemglyph alignment.a search miss, "blocked with no blocker" recovery, per-row context menus.
Verification
fvm flutter analyzeclean acrosslib/andtest/features; 2453 tests greenacross tasks and design-system on the rebased branch. CHANGELOG and flatpak
metainfo updated under 0.9.1064.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation