Skip to content

Redesign task relationship links: one directed choice, editable in place#3563

Merged
matthiasn merged 14 commits into
mainfrom
feat/task-relationship-links-redesign
Jul 24, 2026
Merged

Redesign task relationship links: one directed choice, editable in place#3563
matthiasn merged 14 commits into
mainfrom
feat/task-relationship-links-redesign

Conversation

@matthiasn

@matthiasn matthiasn commented Jul 24, 2026

Copy link
Copy Markdown
Owner

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 blocks link's primary phrase is the word "Blocks", the selected
chip 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.

Before (#3556) After

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.

Before (#3556) After

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 and
a 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 DraggableScrollableSheet that left a large dead gap. The picker body
now shrink-wraps rather than reserving a fixed height under a short list.

Before (#3556) After

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.

Before (#3556) After

Desktop


Shared components

Three design-system changes benefit every caller, not just this feature:

  • DesignSystemDropdown now marks the selected row in single-choice mode — the
    flag was being passed and discarded, so any open single-select list said
    nothing about its current value.
  • DesignSystemSelectionRow exposes titleMaxLines (default unchanged), so
    long titles stop truncating mid-word on the row whose tap commits.
  • EntityPickerSheet takes a rowSize and owns its top inset — every caller
    passes EdgeInsets.zero to control the row indent, which was also welding
    the search field to the header divider.

Testing

  • 2204 tests green across test/features/tasks/, the shared picker/dropdown/
    selection-row suites, the category and label pickers that share those
    components, test/features/journal/repository/, and test/logic/
  • flutter analyze clean project-wide
  • Full patch coverage on new/changed lines

Honest 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

  • New Features
    • Relationship selection is streamlined into a single dropdown combining type and direction.
    • Existing relationships can be edited in place (including changing direction) without unlinking.
    • Linked Tasks now groups sections using the exact selected relationship phrasing, and the card header is shown even when empty.
    • Linking/blocking dialogs now adapt responsively across screen sizes.
  • Bug Fixes
    • Restored support for multiple relationships between tasks.
    • Tasks that previously couldn’t be linked due to empty/duplicate handling can now be linked.
    • Long task titles no longer truncate mid-word during linking.
    • Improved “waiting on”/unresolved blocker messaging and chip presentation.
  • Documentation
    • Updated relationship-linking guidance to match the new dropdown and edit flows.

matthiasn added 11 commits July 24, 2026 23:08
… 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.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@coderabbitai

coderabbitai Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 2bf30eb0-f0a0-4518-aa57-155e6ae3987e

📥 Commits

Reviewing files that changed from the base of the PR and between 961c7d1 and 6c891e0.

📒 Files selected for processing (26)
  • lib/features/journal/repository/journal_repository.dart
  • lib/features/tasks/README.md
  • lib/features/tasks/ui/linked_tasks/edit_link_type_modal.dart
  • lib/features/tasks/ui/linked_tasks/linked_task_row.dart
  • lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart
  • lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart
  • lib/l10n/app_cs.arb
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_cs.dart
  • lib/l10n/app_localizations_da.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart
  • lib/l10n/app_localizations_nl.dart
  • lib/l10n/app_localizations_pt.dart
  • lib/l10n/app_localizations_ro.dart
  • lib/l10n/app_localizations_sv.dart
  • lib/l10n/app_ro.arb
  • test/features/tasks/ui/linked_tasks/edit_link_type_modal_test.dart
  • test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart
🚧 Files skipped from review as they are similar to previous changes (25)
  • lib/features/tasks/ui/linked_tasks/edit_link_type_modal.dart
  • test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart
  • lib/features/journal/repository/journal_repository.dart
  • test/features/tasks/ui/linked_tasks/edit_link_type_modal_test.dart
  • lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart
  • lib/features/tasks/ui/linked_tasks/linked_task_row.dart
  • lib/l10n/app_ro.arb
  • lib/l10n/app_localizations_sv.dart
  • lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart
  • lib/l10n/app_cs.arb
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_en.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_localizations_nl.dart
  • lib/l10n/app_de.arb
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_pt.dart
  • lib/l10n/app_localizations_it.dart
  • lib/features/tasks/README.md
  • lib/l10n/app_localizations_ro.dart
  • lib/l10n/app_es.arb
  • lib/l10n/app_localizations_da.dart
  • lib/l10n/app_localizations_cs.dart

📝 Walkthrough

Walkthrough

The 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.

Changes

Typed relationship linking

Layer / File(s) Summary
Relationship persistence and cycle validation
lib/classes/entry_link.dart, lib/features/journal/repository/..., lib/logic/..., test/features/journal/..., test/logic/...
Link type names are centralized, type-only changes are persisted, existing links can be retyped or reversed, and blocks-cycle checks are shared and tested.
Directed relation picker and task selection
lib/features/tasks/ui/linked_tasks/..., lib/widgets/picker/..., lib/features/design_system/..., test/features/tasks/ui/linked_tasks/...
Relationship type and direction are selected from one dropdown, candidate exclusion uses ExistingRelation, and picker layout, search, title wrapping, and responsive modal behavior are updated.
Linked-task rendering and editing
lib/features/tasks/ui/linked_tasks/..., lib/features/tasks/ui/header/..., test/features/tasks/ui/linked_tasks/..., test/features/tasks/ui/header/...
Linked-task cards remain visible when empty, relationship sections use exact directional phrases, rows support edit and unlink actions, and blocker chips use compact waiting/unresolved labels.
Localization and release metadata
lib/l10n/*, CHANGELOG.md, flatpak/..., lib/features/tasks/README.md, pubspec.yaml
Relationship editing, linked-task empty states, blocker wording, release notes, ADR documentation, and the application version are updated.

Estimated code review effort: 4 (Complex) | ~60 minutes

Possibly related PRs

Suggested reviewers: cyberk1ng

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: task relationships now use a single directed choice and existing links can be edited in place.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/task-relationship-links-redesign

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.

❤️ Share

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

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.22%. Comparing base (0a8f6a6) to head (6c891e0).

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     
Flag Coverage Δ
glados 14.16% <2.37%> (-0.01%) ⬇️
standard 98.97% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 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 win

Clarify the inverse follow-up phrase.

Tiene seguimiento de is awkward and can obscure the intended “has follow-up” direction. Use a clearer phrase such as Tiene 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 value

Consider a toString() for readable failure output.

relationship_type_selector_test.dart lines 66-78 asserts the full 11-element option list; without toString() a mismatch prints Instance 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 win

Assert the carried-over flags, not just id/from/to.

updateLinkType deliberately preserves createdAt, hidden, collapsed, and deletedAt (journal_repository.dart lines 447-452), but no test pins that. Dropping collapsed would 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.

picked is read at pump time, before any interaction, so the returned DirectedRelation? is always null — tests A and B discard it, and the test at lines 223-247 re-inlines the whole pumpWidget just 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 win

Extract the duplicated pickRelation dropdown-driver helper into a shared test utility.

Both files define an identical helper that locates the DesignSystemDropdown and invokes onItemPressed! for a matching label. This same pattern is also referenced in edit_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 this pickRelation implementation 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 win

Extract the repeated row pump into one helper.

Each of the eight tests re-declares the same WidgetTestBench(child: LinkedTaskRow(...)) block, now differing only in manageMode, onEdit, onUnlink, and mediaQueryData. A single pumpRow(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

📥 Commits

Reviewing files that changed from the base of the PR and between 0a8f6a6 and 961c7d1.

📒 Files selected for processing (58)
  • CHANGELOG.md
  • flatpak/com.matthiasn.lotti.metainfo.xml
  • lib/classes/entry_link.dart
  • lib/database/conversions.dart
  • lib/features/design_system/components/dropdowns/design_system_dropdown_panel.dart
  • lib/features/design_system/components/selection/design_system_selection_row.dart
  • lib/features/journal/repository/journal_repository.dart
  • lib/features/tasks/README.md
  • lib/features/tasks/ui/header/desktop_task_header_connector.dart
  • lib/features/tasks/ui/linked_tasks/blocking_task_picker_modal.dart
  • lib/features/tasks/ui/linked_tasks/edit_link_type_modal.dart
  • lib/features/tasks/ui/linked_tasks/link_task_modal.dart
  • lib/features/tasks/ui/linked_tasks/linked_task_row.dart
  • lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart
  • lib/features/tasks/ui/linked_tasks/relationship_type_selector.dart
  • lib/features/tasks/ui/linked_tasks/task_relationship_sections.dart
  • lib/features/tasks/ui/linked_tasks/task_search_picker_body.dart
  • lib/l10n/app_cs.arb
  • lib/l10n/app_da.arb
  • lib/l10n/app_de.arb
  • lib/l10n/app_en.arb
  • lib/l10n/app_es.arb
  • lib/l10n/app_fr.arb
  • lib/l10n/app_it.arb
  • lib/l10n/app_localizations.dart
  • lib/l10n/app_localizations_cs.dart
  • lib/l10n/app_localizations_da.dart
  • lib/l10n/app_localizations_de.dart
  • lib/l10n/app_localizations_en.dart
  • lib/l10n/app_localizations_es.dart
  • lib/l10n/app_localizations_fr.dart
  • lib/l10n/app_localizations_it.dart
  • lib/l10n/app_localizations_nl.dart
  • lib/l10n/app_localizations_pt.dart
  • lib/l10n/app_localizations_ro.dart
  • lib/l10n/app_localizations_sv.dart
  • lib/l10n/app_nl.arb
  • lib/l10n/app_pt.arb
  • lib/l10n/app_ro.arb
  • lib/l10n/app_sv.arb
  • lib/logic/blocks_cycle_guard.dart
  • lib/logic/persistence_entries.dart
  • lib/widgets/picker/entity_picker_sheet.dart
  • pubspec.yaml
  • test/features/design_system/components/dropdowns/design_system_dropdown_test.dart
  • test/features/journal/repository/journal_repository_test.dart
  • test/features/tasks/ui/header/desktop_task_header_connector_test.dart
  • test/features/tasks/ui/linked_tasks/blocking_task_picker_modal_test.dart
  • test/features/tasks/ui/linked_tasks/edit_link_type_modal_test.dart
  • test/features/tasks/ui/linked_tasks/link_task_modal_test.dart
  • test/features/tasks/ui/linked_tasks/linked_task_row_test.dart
  • test/features/tasks/ui/linked_tasks/linked_tasks_widget_actions_test.dart
  • test/features/tasks/ui/linked_tasks/linked_tasks_widget_test.dart
  • test/features/tasks/ui/linked_tasks/relationship_type_selector_test.dart
  • test/features/tasks/ui/linked_tasks/task_relationship_sections_test.dart
  • test/features/tasks/ui/widgets/task_manual_screenshots_test.dart
  • test/logic/blocks_cycle_guard_test.dart
  • test/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

Comment thread lib/features/journal/repository/journal_repository.dart
Comment thread lib/features/tasks/README.md
Comment thread lib/features/tasks/ui/linked_tasks/edit_link_type_modal.dart
Comment thread lib/features/tasks/ui/linked_tasks/linked_task_row.dart Outdated
Comment thread lib/features/tasks/ui/linked_tasks/linked_tasks_widget.dart
Comment thread lib/l10n/app_localizations_es.dart Outdated
Comment thread lib/l10n/app_localizations_fr.dart Outdated
Comment thread lib/l10n/app_localizations_fr.dart Outdated
Comment thread lib/l10n/app_localizations.dart
Comment thread lib/l10n/app_ro.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.
@matthiasn
matthiasn merged commit 362fd0c into main Jul 24, 2026
31 of 32 checks passed
@matthiasn
matthiasn deleted the feat/task-relationship-links-redesign branch July 24, 2026 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant