Redesign task relationship links: one directed choice, editable in place#3563
Conversation
… chip wording Adds the ability to retype or flip the direction of an existing typed relationship link in place (EditLinkTypeModal, JournalRepository.updateLinkType), extracts the blocks-cycle guard so both create and edit paths share it, and fixes _hasChange to actually detect a link-type-only edit. Also fixes three real issues surfaced by reviewing PR #3556's screenshots: LinkTaskModal/BlockingTaskPickerModal no longer use an untested DraggableScrollableSheet reveal (removes a real dead-space gap above the sheet), the header's blocked-by chip no longer repeats "Blocked" next to the status pill ("Waiting on {title}" instead), and the new edit-relationship icon was changed from a pencil to a swap glyph to avoid colliding with the Groomed status icon.
…modal Replaces the bespoke DraggableScrollableSheet + hand-rolled task list in LinkTaskModal, BlockingTaskPickerModal, and EditLinkTypeModal with ModalUtils.showSinglePageModal + the shared EntityPickerSheet, matching every other task-detail picker (status/priority/category/project/due-date). Fixes the non-adaptive dialog-vs-bottom-sheet gap flagged as the top finding in design-review-panel round 1. Also: adds a caption to RelationshipTypeSelector's direction toggle, sweeps remaining hardcoded spacing to design tokens, fixes two alignment bugs, and switches the blocked-by chip from the error to the warning color token so it doesn't double up with the overdue due-date chip's red.
…der, toggle wording EntityPickerSheet reserved a fixed maxHeight regardless of row count, so a short result list left the same kind of blank picker surface the responsive modal fix in the previous commit was meant to eliminate — shrink-wrap the list (capped at the existing maxHeight) instead. Also: give the flat/plain-link rows on the linked-tasks card their own "Other links" section header so they no longer read as an unlabeled continuation of the typed section above them; add a chevron to the blocked-by chip so it reads as tappable; and reframe the relationship-type direction toggle with arrows so it stops repeating the just-selected type pill's exact wording (e.g. "Blocks" / "Blocks"). Shortened the blocker-picker modal title so its header height matches its sibling modals.
…on colour, chevron parity The "Other links" header added last round fixed the section-level ambiguity but the flat rows themselves still borrowed the typed-relationship treatment (direction glyph + coloured caption reading a content-free "to"/"from"), so the mode error simply moved down a level. Plain links now render captionless, matching how the split Blocks/Blocked-by sections already render. Also: the link-direction caption no longer borrows alert.info/alert.success, which are already spoken for as task-status colours on the same card; a captioned row gets a third line before ellipsizing so long titles stop truncating mid-word on phone; the blocked-by chip and linked rows now share one chevron-size constant instead of two mismatched literals; the multi-blocker sheet stops double-indenting its rows; and the direction toggle's caption reads "This task…" so the control completes a sentence rather than restating the relationship pill above it.
…ent, neutral badge
Every typed relationship now gets its own direction-specific section titled
with that direction's phrase ("Follows up on" / "Has follow-up"), replacing
the merged bidirectional sections that needed a per-row caption. That caption
ate ~40% of the row width on phone, wrapped long titles to three lines, and
left one decorated row template sitting next to a plain one — so it is gone
entirely, along with LinkedTaskRowData.direction/caption. Every row on the
card now renders identically: status glyph, title, trailing affordance.
With rows uniformly neutral, the signal moves to the header rung: "Blocked by"
alone carries an amber accent and a leading glyph, tying the card to the
task header's own "Waiting on X" chip. It is the only colour on the card, so
it reads as signal rather than as one more competing hue.
Also: the count badge drops alert.info (blue already means In Progress on the
same card) for a neutral fill sized from a spacing token; section headers bind
to the rows they label; and the now-unused merged-section and to/from caption
strings are removed from all locales.
…direction
Type and direction were two controls for one value. For 4 of the 5 directional
types the selected type pill and the direction toggle's first segment showed
the identical word ("Blocks" above "Blocks"), which reviewers and test users
read as a duplicated or contradictory control across three review rounds —
rewording it twice did not help, because the split itself was the problem.
Every established issue tracker presents relations as a single flat list of
directed phrases, so this does too: one dropdown completing the sentence
"This task… ⟨Blocks⟩", listing all eleven relations (the symmetric link plus
each type both ways). DirectedRelation carries type + direction together, so
callers no longer reconcile two independent values, and the plain link now
reads "Relates to" to complete the same sentence stem as the rest.
Also: the card always renders its header with an explicit link action. It
previously rendered nothing at all when a task had no links, and the only
call site for the link modal sat inside that hidden card's overflow menu — so
a task with no links had no reachable way to link one. Rows also stay
navigable in manage mode, where nulling the row tap only produced a row that
looked tappable and wasn't.
…ce, real desktop measure
The round that removed a duplicated control shipped with the link action
duplicated: the new header add_link button sat ~60px from a 'Link existing
task…' item in the same header's overflow menu. The menu item is gone; the
header button is the entry point.
Vocabulary now has one source. Section headers are derived from the same
phrase pair the picker offers, so the card reads back exactly the words that
were picked ("Is blocked by", not a hand-written "Blocked by" sitting beside
a derived "Is duplicated by"), and the plain-link section is headed "Relates
to" — the picker's own word for it — instead of an unrelated "Other links".
That header now only renders when there are typed links to be distinguished
from. The six orphaned linkType*Option strings and the dead
relationshipTypeOptionLabel go with it.
The desktop screenshots have been misleading for five rounds: the capture
rendered the card unconstrained at 1280 while the task detail page caps
content at kDetailContentMaxWidth, so reviewers kept flagging a huge
horizontal void that does not exist in the app. The harness now captures at
the shipped measure, and captures the zero-link card, which had never been
screenshotted at all.
Also pulled forward from the follow-up backlog: picker rows cap task titles
at two lines rather than one, so a title no longer truncates mid-word on the
row whose tap commits the link (new titleMaxLines on the shared selection row
and picker sheet, default unchanged for every existing caller); and pickers
carry their own top inset, since every caller zeroes the modal padding to
control the row indent and was welding the search field to the header divider.
LinkedTaskRow hand-rolled InkWell + Row + Padding, so the same task title rendered at a different size and gutter on the card than in the picker one tap away, and the row sat below the design system's own minimum interactive height. It now composes DesignSystemListItem, which settles all three at once. The status label moves onto the row as a trailing anchor rather than a second line: it keeps the row one line tall, surfaces state the card previously hid, and on a wide detail pane it gives the row something at its right edge so the chevron no longer floats alone across an otherwise empty span. Manage-mode actions get the design system's minimum interactive target instead of a 32px literal, a gap between them, and the destructive one carries more weight than its neighbour so the two stop reading as interchangeable smudges. Also: single-choice dropdowns now mark their selected row. The flag was being passed and discarded, so an open list of similar-sounding options said nothing about what was already chosen — worst on the edit path, which is opened precisely to read that state back. Fixed in the shared component, so every single-select dropdown in the app gains it.
The link picker excluded every task the anchor already touched, but the schema is UNIQUE(from_id, to_id, type) — one pair may legitimately hold a plain link and a blocks edge and a follow-up. Linking A to B a second way was therefore impossible, and the flow reported it as "No tasks found". Exclusion is now per-relation: the candidate list drops only the tasks that would duplicate the relation currently selected, and recomputes as that selection changes. Direction counts, so the inverse of an existing link stays offerable. Design fixes from round 6: - The zero-link card was the round's headline change and shipped without ever being screenshotted; captured, it was an empty bordered box whose entire UI was two identically-coloured glyphs. It is now a worded action row with a line saying what a link is for, and the header drops its icon-only duplicate of that action until there is a list to add to. - Card and picker rows now share one rank. Round 5 asked for this and round 6 only half-delivered: the card moved to the DS list item while the picker stayed a size larger, so the same task title still rendered at two ranks one tap apart. EntityPickerSheet takes a rowSize, matching the card. - The trailing status label works where there is width to separate it from the title and fought the title for the same line on a phone, pushing every row to two lines. It stays trailing on wide panes and drops to the subtitle slot on narrow ones. - Section labels share one left rail: the accent glyph now occupies a reserved column that plain headers leave empty, instead of the most important label being the only one off-grid. Headers also bind closer to the rows they label.
Swept into the branch by an over-broad 'git add -A test/'. It is a disposable capture harness for design review, not a test of the app — the app-screenshots convention is to delete it once the images are shared. Kept on disk while the design rounds are still running; it must not reach the PR.
README rewritten for what actually ships: relationship type and direction are one choice, the sections are direction-specific and titled with the phrase the picker offered, the card always renders its header, and exclusion is per-relation because a pair may hold several. CHANGELOG and the flatpak metainfo amend the existing 0.9.1062 entry rather than adding a new one — the typed-relationships feature is described there and has not shipped yet, so this refines that description instead of logging fixes for defects no user ever saw.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (26)
🚧 Files skipped from review as they are similar to previous changes (25)
📝 WalkthroughWalkthroughThe PR introduces directed relationship selection and in-place relationship editing, adds shared blocks-cycle validation, updates linked-task rendering and responsive picker layouts, changes blocker presentation, refreshes localization and documentation, and bumps the release version to 0.9.1063. ChangesTyped relationship linking
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3563 +/- ##
=======================================
Coverage 99.22% 99.22%
=======================================
Files 1772 1774 +2
Lines 130059 130106 +47
=======================================
+ Hits 129045 129093 +48
+ Misses 1014 1013 -1
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:
|
Embedding the blocker's title made the chip grow with it — on a realistic title it spanned the header on its own line and out-shouted the status pill beside it, which is the opposite of what a derived, secondary fact should do. The header's job is that the task is waiting; which task it waits on is one glance away in the Linked Tasks card, where 'Is blocked by' now leads and names it. The chip reads 'Waiting on 1 task' / 'Waiting on 3 tasks', keeps its tap target, and the tooltip still names the single blocker at no layout cost.
0.9.1062 is already tagged and shipped (0.9.1062+4242 contains #3556), so its CHANGELOG and metainfo entries are release history and must not be rewritten — an earlier commit on this branch had amended them in place. Those are restored verbatim from the tag, and this work gets its own 0.9.1063 section describing what changed relative to what shipped.
There was a problem hiding this comment.
Actionable comments posted: 16
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_es.arb (1)
2799-2800: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winClarify the inverse follow-up phrase.
Tiene seguimiento deis awkward and can obscure the intended “has follow-up” direction. Use a clearer phrase such asTiene una tarea de seguimiento, adjusted to match how the linked task title is rendered.🤖 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_es.arb` around lines 2799 - 2800, Update the linkPhraseFollowsUpInverse localization value to use a clear Spanish phrase expressing “has a follow-up task,” such as “Tiene una tarea de seguimiento,” and ensure its wording fits the linked task title rendering. Leave linkPhraseFollowsUpPrimary unchanged.
🧹 Nitpick comments (5)
lib/features/tasks/ui/linked_tasks/relationship_type_selector.dart (1)
27-48: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a
toString()for readable failure output.
relationship_type_selector_test.dartlines 66-78 asserts the full 11-element option list; withouttoString()a mismatch printsInstance of 'DirectedRelation'eleven times.🔍 Suggested addition
`@override` int get hashCode => Object.hash(type, inverse); + + `@override` + String toString() => 'DirectedRelation($id)'; }🤖 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/relationship_type_selector.dart` around lines 27 - 48, Add a readable toString() override to DirectedRelation that includes its EntryLinkType and inverse state, so option-list assertion failures identify each relation distinctly instead of showing only the class name.test/features/journal/repository/journal_repository_test.dart (1)
3138-3185: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the carried-over flags, not just id/from/to.
updateLinkTypedeliberately preservescreatedAt,hidden,collapsed, anddeletedAt(journal_repository.dart lines 447-452), but no test pins that. Droppingcollapsedwould silently re-expand a collapsed link with no failing test.♻️ Suggested additions to the existing retype test
final existing = EntryLink.followsUp( id: 'link-id', fromId: 'from-id', toId: 'to-id', createdAt: DateTime(2023), updatedAt: DateTime(2023), vectorClock: const VectorClock({'node1': 0}), + collapsed: true, + hidden: true, );expect(persisted.id, 'link-id'); expect(persisted.fromId, 'from-id'); expect(persisted.toId, 'to-id'); + // Retyping must not reset the row's existing flags/creation time. + expect(persisted.createdAt, DateTime(2023)); + expect(persisted.collapsed, isTrue); + expect(persisted.hidden, isTrue);As per coding guidelines: "Every test must assert meaningful behavior such as displayed values, state changes, callbacks, or error handling."
🤖 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/journal/repository/journal_repository_test.dart` around lines 3138 - 3185, Strengthen the existing “retypes an existing link in place” test by initializing the source EntryLink with representative createdAt, hidden, collapsed, and deletedAt values, then assert the persisted link retains each value after updateLinkType. Keep the existing type, identity, direction, and sync assertions unchanged.Source: Coding guidelines
test/features/tasks/ui/linked_tasks/relationship_type_selector_test.dart (1)
167-181: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
pumpSelector's return value can never be non-null, which forces test C to duplicate the pump.
pickedis read at pump time, before any interaction, so the returnedDirectedRelation?is alwaysnull— tests A and B discard it, and the test at lines 223-247 re-inlines the wholepumpWidgetjust to observe the callback. Returning a mutable holder lets all three share one pump path.♻️ Suggested helper reshape
- Future<DirectedRelation?> pumpSelector( + Future<List<DirectedRelation>> pumpSelector( WidgetTester tester, { DirectedRelation selected = const DirectedRelation(EntryLinkType.basic), }) async { - DirectedRelation? picked; + final picked = <DirectedRelation>[]; await tester.pumpWidget( WidgetTestBench( child: RelationshipTypeSelector( selected: selected, - onChanged: (relation) => picked = relation, + onChanged: picked.add, ), ), ); return picked; }Test C then becomes:
testWidgets('reports the picked option as a type + direction pair', ( tester, ) async { final picked = await pumpSelector(tester); final dropdown = tester.widget<DesignSystemDropdown>( find.byType(DesignSystemDropdown), ); dropdown.onItemPressed!( dropdown.items.firstWhere((i) => i.label == 'Is blocked by'), ); expect(picked, [ const DirectedRelation(EntryLinkType.blocks, inverse: true), ]); });As per coding guidelines: "Extract shared widget pump/setup helpers, repeated mock stubs, and complex state into helpers or test-bench classes; pass varying values as named parameters."
🤖 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/relationship_type_selector_test.dart` around lines 167 - 181, Reshape pumpSelector to return a mutable collection of selected DirectedRelation values initialized before pumpWidget, and append each onChanged callback result to it. Update callers to use the returned collection, including the interaction test, so all tests share the helper without duplicating widget setup and can observe callbacks after interaction.Source: Coding guidelines
test/features/tasks/ui/linked_tasks/link_task_modal_test.dart (1)
80-91: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the duplicated
pickRelationdropdown-driver helper into a shared test utility.Both files define an identical helper that locates the
DesignSystemDropdownand invokesonItemPressed!for a matching label. This same pattern is also referenced inedit_link_type_modal_test.dart. A shared helper (e.g. in a test utils file) would avoid drift between copies as the dropdown interaction contract evolves.
test/features/tasks/ui/linked_tasks/link_task_modal_test.dart#L80-L91: extract thispickRelationimplementation into a shared test helper and import it here.test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart#L152-L163: replace this duplicate with the shared helper.As per path instructions: "Extract shared widget pump/setup helpers, repeated mock stubs, and complex state into helpers or test-bench classes; pass varying values as named parameters."
🤖 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/link_task_modal_test.dart` around lines 80 - 91, The duplicated pickRelation dropdown driver should become one shared test utility. In test/features/tasks/ui/linked_tasks/link_task_modal_test.dart lines 80-91, move the helper that locates DesignSystemDropdown and invokes onItemPressed for the matching label into a shared test utility, then import and use it; in test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart lines 152-163, remove the duplicate local helper and use the shared utility. Ensure edit_link_type_modal_test.dart also reuses the same helper where applicable.Source: Path instructions
test/features/tasks/ui/linked_tasks/linked_task_row_test.dart (1)
102-148: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the repeated row pump into one helper.
Each of the eight tests re-declares the same
WidgetTestBench(child: LinkedTaskRow(...))block, now differing only inmanageMode,onEdit,onUnlink, andmediaQueryData. A singlepumpRow(tester, {...})collapses that and keeps future constructor changes to one edit.As per coding guidelines: "Extract shared widget pump/setup helpers, repeated mock stubs, and complex state into helpers or test-bench classes; pass varying values as named parameters."
♻️ Shared pump helper
Future<void> pumpRow( WidgetTester tester, { bool manageMode = false, Future<void> Function()? onEdit, Future<void> Function()? onUnlink, MediaQueryData? mediaQueryData, }) => tester.pumpWidget( WidgetTestBench( mediaQueryData: mediaQueryData, child: LinkedTaskRow( taskId: 'anchor-task', data: buildRowData(), manageMode: manageMode, onEdit: onEdit, onUnlink: onUnlink, ), ), );🤖 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_task_row_test.dart` around lines 102 - 148, Extract the repeated WidgetTestBench and LinkedTaskRow setup into a shared pumpRow helper in the linked-task row tests, with named parameters for manageMode, onEdit, onUnlink, and mediaQueryData and the existing defaults. Replace all eight duplicated pumpWidget blocks with this helper while preserving each test’s varying configuration and assertions.Source: Path instructions
🤖 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/journal/repository/journal_repository.dart`:
- Around line 380-381: Update the documentation for debugHasChange to reflect
that it now compares seven link fields, including the entry link type name via
entryLinkTypeName, rather than describing it as a pure six-field comparator;
leave the comparison logic unchanged.
In `@lib/features/tasks/README.md`:
- Around line 677-681: Update the earlier candidate-filter description around
the flow documentation to state that candidates exclude only tasks holding the
currently selected relation, recalculated when the selection changes. Remove the
claim that all tasks already linked to the anchor are excluded, while preserving
the documented support for multiple relation types and direction-specific
inverse links.
In `@lib/features/tasks/ui/linked_tasks/edit_link_type_modal.dart`:
- Around line 93-121: Update the Save handler in buildPickerApplyFooter to
serialize submissions: track an in-flight state, ignore subsequent taps while
updateLinkType is pending, and preserve that state until the operation
completes. Disable the Save button during the request and reset the state on
completion, while keeping the existing success, failure, and navigation
behavior.
In `@lib/features/tasks/ui/linked_tasks/linked_task_row.dart`:
- Around line 71-94: Replace the window-width check assigned to
wideEnoughForTrailingStatus with LayoutBuilder constraints, using
constraints.maxWidth >= kDesktopBreakpoint to determine placement. Keep the
existing subtitle and trailing conditions unchanged so the decision reflects the
row’s actual constrained width rather than MediaQuery.sizeOf(context).width.
In `@lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart`:
- Around line 378-394: In the new-task relationship flow around
selection.inverse and PersistenceLogic.createLink, create the typed link first
and inspect its boolean result. Only remove the redundant BasicLink via
journalRepositoryProvider.removeTypedLink after successful creation; when
creation is rejected or fails, preserve the BasicLink and surface the existing
link-blocking error behavior to the user.
In `@lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart`:
- Around line 209-238: Update the header Row containing the accent glyph and
label so the Text can shrink within the available width, and cap the label to a
single line with ellipsis overflow. Preserve the reserved glyph column, spacing,
and existing styling in the surrounding header construction.
In `@lib/l10n/app_cs.arb`:
- Line 4143: Update the taskBlockerPickerTitle localization value from “Co to
blokuje?” to “Co blokuje tento úkol?” so it asks what is blocking the task.
In `@lib/l10n/app_de.arb`:
- Line 4136: Update the taskBlockerPickerTitle localization value to explicitly
reference the task, using the German wording “Was blockiert diese Aufgabe?”
instead of the ambiguous pronoun.
In `@lib/l10n/app_es.arb`:
- Line 4123: Update the =1 Spanish translation in taskBlockedByChipTooltip to
remove the personal “a”, changing the wording to “Esperando {title}” while
leaving the plural variant unchanged.
In `@lib/l10n/app_fr.arb`:
- Line 2792: Update the French localization value for linkPhraseBasic from the
masculine form to the feminine form “Est liée à”, matching the grammatical
gender used by the related inverse phrases.
- Line 4136: Update the taskBlockerPickerTitle localization value to explicitly
refer to the task by using “Qu'est-ce qui bloque cette tâche ?” instead of the
ambiguous current wording.
In `@lib/l10n/app_localizations_es.dart`:
- Around line 10292-10293: Update the taskBlockerPickerTitle localization string
to use “¿Qué está bloqueando esta tarea?” instead of the vague current wording,
preserving the existing getter and override.
In `@lib/l10n/app_localizations_fr.dart`:
- Line 6723: Update the ARB source for the basic relationship phrase so the
translation uses “Est liée à” for the feminine subject, then rerun make l10n to
regenerate linkPhraseBasic in app_localizations_fr.dart. Do not edit the
generated Dart file directly.
- Line 10334: Update the taskBlockerPickerTitle localization to explicitly refer
to the task using “Qu'est-ce qui bloque cette tâche ?”, then update the ARB
source and rerun make l10n so generated localization files remain synchronized.
In `@lib/l10n/app_localizations.dart`:
- Around line 17493-17497: Update the ARB source for taskBlockerPickerTitle to
use the documented “What’s blocking this task?” copy, then regenerate
app_localizations.dart so the generated getter reflects the updated
localization.
In `@lib/l10n/app_ro.arb`:
- Around line 2789-2790: Reorder the linkedTasksEmptyAction and
linkedTasksEmptyHint entries in the ARB file so they appear before
linkedTasksMenuTooltip, preserving their translations, then run make
sort_arb_files to apply the repository’s standard ARB ordering.
---
Outside diff comments:
In `@lib/l10n/app_es.arb`:
- Around line 2799-2800: Update the linkPhraseFollowsUpInverse localization
value to use a clear Spanish phrase expressing “has a follow-up task,” such as
“Tiene una tarea de seguimiento,” and ensure its wording fits the linked task
title rendering. Leave linkPhraseFollowsUpPrimary unchanged.
---
Nitpick comments:
In `@lib/features/tasks/ui/linked_tasks/relationship_type_selector.dart`:
- Around line 27-48: Add a readable toString() override to DirectedRelation that
includes its EntryLinkType and inverse state, so option-list assertion failures
identify each relation distinctly instead of showing only the class name.
In `@test/features/journal/repository/journal_repository_test.dart`:
- Around line 3138-3185: Strengthen the existing “retypes an existing link in
place” test by initializing the source EntryLink with representative createdAt,
hidden, collapsed, and deletedAt values, then assert the persisted link retains
each value after updateLinkType. Keep the existing type, identity, direction,
and sync assertions unchanged.
In `@test/features/tasks/ui/linked_tasks/link_task_modal_test.dart`:
- Around line 80-91: The duplicated pickRelation dropdown driver should become
one shared test utility. In
test/features/tasks/ui/linked_tasks/link_task_modal_test.dart lines 80-91, move
the helper that locates DesignSystemDropdown and invokes onItemPressed for the
matching label into a shared test utility, then import and use it; in
test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart lines
152-163, remove the duplicate local helper and use the shared utility. Ensure
edit_link_type_modal_test.dart also reuses the same helper where applicable.
In `@test/features/tasks/ui/linked_tasks/linked_task_row_test.dart`:
- Around line 102-148: Extract the repeated WidgetTestBench and LinkedTaskRow
setup into a shared pumpRow helper in the linked-task row tests, with named
parameters for manageMode, onEdit, onUnlink, and mediaQueryData and the existing
defaults. Replace all eight duplicated pumpWidget blocks with this helper while
preserving each test’s varying configuration and assertions.
In `@test/features/tasks/ui/linked_tasks/relationship_type_selector_test.dart`:
- Around line 167-181: Reshape pumpSelector to return a mutable collection of
selected DirectedRelation values initialized before pumpWidget, and append each
onChanged callback result to it. Update callers to use the returned collection,
including the interaction test, so all tests share the helper without
duplicating widget setup and can observe callbacks after interaction.
🪄 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: 55ef57fa-87e1-437a-bdbb-d689297327c5
📒 Files selected for processing (58)
CHANGELOG.mdflatpak/com.matthiasn.lotti.metainfo.xmllib/classes/entry_link.dartlib/database/conversions.dartlib/features/design_system/components/dropdowns/design_system_dropdown_panel.dartlib/features/design_system/components/selection/design_system_selection_row.dartlib/features/journal/repository/journal_repository.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_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/logic/blocks_cycle_guard.dartlib/logic/persistence_entries.dartlib/widgets/picker/entity_picker_sheet.dartpubspec.yamltest/features/design_system/components/dropdowns/design_system_dropdown_test.darttest/features/journal/repository/journal_repository_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/relationship_type_selector_test.darttest/features/tasks/ui/linked_tasks/task_relationship_sections_test.darttest/features/tasks/ui/widgets/task_manual_screenshots_test.darttest/logic/blocks_cycle_guard_test.darttest/widgets/picker/entity_picker_sheet_test.dart
💤 Files with no reviewable changes (5)
- lib/l10n/app_da.arb
- lib/l10n/app_sv.arb
- lib/l10n/app_it.arb
- lib/l10n/app_nl.arb
- lib/l10n/app_pt.arb
…onstraints, l10n Data integrity: - _createNewLinkedTask deleted the auto-created BasicLink before creating the typed one and ignored createLink's result, so a cycle-guard rejection left the freshly created task with no link back to its parent and no message. Create first, delete only on success, and surface the rejection otherwise. - EditLinkTypeModal's Save computes swapDirection against the link's pre-edit direction, so a second tap while the first was in flight recomputed the same flip and reversed the link back. Commits are now serialised. Layout: - LinkedTaskRow chose its status placement from the window width, but the card is capped at the detail reading measure on wide windows and can also sit in a narrow column — so the window said nothing about the row's own room. It now measures its own constraints. - Section header labels ellipsize instead of overflowing once the glyph column and both insets are taken off a phone-width card. Localization: fixed a direction reversal in Czech (the old wording read as "what does it block"), feminine agreement in French, and a personal "a" before a task title in Spanish. The blocker-picker title keeps its short form in every locale but now uses a pronoun that agrees with the noun, which answers the ambiguity without the longer explicit phrasing — that wraps the modal title to two lines and detaches the close button, verified by capture. Docs: debugHasChange's comment counted six fields after it gained a seventh, and the README's flow section still claimed every already-linked task is excluded.
Follow-up to #3556 (ADR 0042 typed relationship links). Reviewing that PR's
screenshots, the result read as unorganised rather than high-end, so this is a
redesign pass over the same feature plus one new capability: changing an
existing link's relationship in place, instead of unlink-and-recreate.
Six rounds of grounded design review drove this (expert panel + user personas,
scoring real screenshots). Two of the most valuable findings turned out to be
functional bugs rather than taste.
The main change: relationship type and direction are one choice
The picker was a row of six type chips plus a separate primary/inverse toggle.
Because a
blockslink's primary phrase is the word "Blocks", the selectedchip and the toggle's first segment showed the identical word stacked a few
pixels apart — for four of the five directional types. Reviewers and test users
read it as a duplicated or contradictory control in three consecutive rounds;
rewording it twice didn't help, because the split itself was the defect.
Established issue trackers present relations as one flat list of directed
phrases. This now does too — a single control completing the sentence
"This task… ⟨Blocks⟩", listing all eleven relations.
The full list, with the current value marked:
The Linked Tasks card
Sections are now direction-specific and titled with the very phrase the picker
offered, so the card reads back the exact words you chose. Every row uses one
template. "Is blocked by" leads and is the only accented thing on the card.
At desktop width, with the status label as a trailing anchor:
New: edit a relationship in place
Reverse which task is the blocker, or change the relationship entirely, without
unlinking and starting over.
Two functional bugs fixed
A task with no links had no way to link one. The card rendered nothing at
zero links, and the link modal's only call site lived inside that hidden card's
overflow menu. It now always renders its header, with a worded action.
A pair of tasks could only ever hold one relationship. The schema is
UNIQUE(from_id, to_id, type)— a pair may legitimately hold a plain link anda blocks edge and a follow-up — but the picker excluded every task the anchor
already touched, so a second relationship was impossible and the UI reported it
as "No tasks found". Exclusion is now per-relation and recomputes as the
selection changes; direction counts, so the inverse of an existing link stays
offerable.
Modals and the blocked-by chip
Three modals moved to the shared responsive modal every other task-detail
picker uses (bottom sheet under 560pt, centred dialog above), replacing a
bespoke
DraggableScrollableSheetthat left a large dead gap. The picker bodynow shrink-wraps rather than reserving a fixed height under a short list.
The header chip moved off error-red (already owned by the overdue due-date
chip) onto the warning token and gained a tap affordance. It also stopped
embedding the blocker's title: the title made the chip grow with it, so on a
realistic title it spanned the header on its own line and out-shouted the
status pill beside it — the opposite of what a derived, secondary fact should
do. The header's job is that the task is waiting; which task it waits on
is one glance away in the Linked Tasks card, where "Is blocked by" now leads
and names it. The tooltip still names the single blocker, at no layout cost.
Desktop
Shared components
Three design-system changes benefit every caller, not just this feature:
DesignSystemDropdownnow marks the selected row in single-choice mode — theflag was being passed and discarded, so any open single-select list said
nothing about its current value.
DesignSystemSelectionRowexposestitleMaxLines(default unchanged), solong titles stop truncating mid-word on the row whose tap commits.
EntityPickerSheettakes arowSizeand owns its top inset — every callerpasses
EdgeInsets.zeroto control the row indent, which was also weldingthe search field to the header divider.
Testing
test/features/tasks/, the shared picker/dropdown/selection-row suites, the category and label pickers that share those
components,
test/features/journal/repository/, andtest/logic/flutter analyzeclean project-wideHonest status
The design panel's bar was 8/10 on both panels; this lands at 7.17 experts /
7.0 personas, up from 6.0 at the first round, and the first round where no
expert scored below 7. It did not clear the bar. The remaining gap is
concentrated in two tracked follow-ups rather than polish: there is no undo
after a one-tap commit (a standing blocker for the low-vision and novice
personas), and manage mode is still a hidden mode with no inline exit. Both are
interaction additions with new copy, better as a focused follow-up than bolted
onto this PR.
Also worth noting for anyone reading the review history: for five rounds the
desktop screenshots were captured unconstrained at 1280pt while the task detail
page caps content at 960pt, so a slice of the earlier desktop feedback was
measuring the test harness rather than the product. Fixed; the desktop shots
above are at the shipped measure.
Summary by CodeRabbit