From 5b3fbb994f41b07ad5e039b42869c75a32216809 Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 1 Jun 2026 15:26:23 +0200 Subject: [PATCH] move the pin toggle to the pane leading slot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pin/unpin toggle is a mode control, not navigation — grouping it with back/forward/jump implied they work alike. Pull it out of ReaderActionBar into a standalone ReaderPinButton placed before the title (ClidePaneChrome.leading), leaving the right-hand navigator to back/forward/jump-to-pin/edit. Applies to all three readers. (T-198) Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 9 ++-- .../decisions/src/decision_detail_view.dart | 5 +- lib/builtin/markdown/src/markdown_viewer.dart | 5 +- lib/builtin/shared/reader_chrome.dart | 52 +++++++++++-------- .../tickets/src/ticket_detail_view.dart | 5 +- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f977faa0..1625ca69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,10 +40,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. `.clideignore` pair — the single ignore knob clide owns. (T-52) - `files.walk` command — a recursive, ignore-pruned, capped flat file listing of the workspace, backing quick-open and search. (T-51, T-52) -- Sidebar readers (markdown, decision, and ticket) gain a chrome action bar: a - pin/unpin toggle on the left, then a navigator (back, forward, jump-to-pin) and - an edit pencil on the right (no pencil for tickets — they're not files). The - ticket reader joins the shared retained nav and drops its per-click tab churn. +- Sidebar readers (markdown, decision, and ticket) gain chrome: a pin/unpin + toggle in the leading slot before the title (a mode control, kept separate + from navigation), and a right-hand navigator — back, forward, jump-to-pin — + plus an edit pencil (no pencil for tickets — they're not files). The ticket + reader joins the shared retained nav and drops its per-click tab churn. (T-189, T-190, T-191, T-198, T-199) ### Fixed diff --git a/lib/builtin/decisions/src/decision_detail_view.dart b/lib/builtin/decisions/src/decision_detail_view.dart index 25f32632..47468a41 100644 --- a/lib/builtin/decisions/src/decision_detail_view.dart +++ b/lib/builtin/decisions/src/decision_detail_view.dart @@ -110,6 +110,10 @@ class _DecisionDetailViewState extends State { return ClidePaneChrome( title: id, subtitle: title, + leading: ReaderPinButton( + pinned: _nav?.hasPinned ?? false, + onTap: _decision != null ? _onPin : null, + ), trailing: [ ReaderActionBar( canGoBack: _nav?.canGoBack ?? false, @@ -117,7 +121,6 @@ class _DecisionDetailViewState extends State { hasPinned: _nav?.hasPinned ?? false, onBack: (_nav?.canGoBack ?? false) ? _onBack : null, onForward: (_nav?.canGoForward ?? false) ? _onForward : null, - onPin: _decision != null ? _onPin : null, onJumpToPin: (_nav?.hasPinned ?? false) ? _onJumpToPin : null, onEdit: filePath != null ? _onEdit : null, ), diff --git a/lib/builtin/markdown/src/markdown_viewer.dart b/lib/builtin/markdown/src/markdown_viewer.dart index fb5b50c7..d0ef13d7 100644 --- a/lib/builtin/markdown/src/markdown_viewer.dart +++ b/lib/builtin/markdown/src/markdown_viewer.dart @@ -103,6 +103,10 @@ class _MarkdownViewerState extends State { return ClidePaneChrome( title: _path ?? 'viewer', subtitle: '${_content!.split('\n').length} lines', + leading: ReaderPinButton( + pinned: _nav?.hasPinned ?? false, + onTap: _path != null ? _onPin : null, + ), trailing: [ ReaderActionBar( canGoBack: _nav?.canGoBack ?? false, @@ -110,7 +114,6 @@ class _MarkdownViewerState extends State { hasPinned: _nav?.hasPinned ?? false, onBack: (_nav?.canGoBack ?? false) ? _onBack : null, onForward: (_nav?.canGoForward ?? false) ? _onForward : null, - onPin: _path != null ? _onPin : null, onJumpToPin: (_nav?.hasPinned ?? false) ? _onJumpToPin : null, onEdit: _path != null ? _onEdit : null, ), diff --git a/lib/builtin/shared/reader_chrome.dart b/lib/builtin/shared/reader_chrome.dart index d847214a..051ea059 100644 --- a/lib/builtin/shared/reader_chrome.dart +++ b/lib/builtin/shared/reader_chrome.dart @@ -14,13 +14,12 @@ import 'package:flutter/widgets.dart'; // Action bar widget // --------------------------------------------------------------------------- -/// A row of reader-chrome action buttons. Layout (T-196 UX): a pin/unpin -/// **toggle on the left**, then the **navigator** on the right — back, -/// forward, jump-to-pin (only while pinned) — and the edit pencil last. -/// The left toggles pinned state; the right navigates. -/// -/// Designed to plug into [ClidePaneChrome.trailing]. All callbacks are -/// optional — pass null to hide/disable the corresponding button. +/// The reader's **navigator** — back, forward, jump-to-pin (only while +/// pinned), and the edit pencil. Plugs into [ClidePaneChrome.trailing]. +/// The pin/unpin toggle is deliberately NOT here: it's a mode control, +/// not navigation, so it lives in the pane's leading slot as a +/// standalone [ReaderPinButton] (T-198). All callbacks are optional — +/// pass null to hide/disable the corresponding button. class ReaderActionBar extends StatelessWidget { const ReaderActionBar({ super.key, @@ -29,7 +28,6 @@ class ReaderActionBar extends StatelessWidget { required this.hasPinned, required this.onBack, required this.onForward, - required this.onPin, required this.onJumpToPin, required this.onEdit, }); @@ -40,10 +38,6 @@ class ReaderActionBar extends StatelessWidget { final VoidCallback? onBack; final VoidCallback? onForward; - /// Called when the pin toggle is tapped — pins the current entry when - /// nothing is pinned, else clears the pin. - final VoidCallback? onPin; - /// Called when the jump-to-pin button (in the navigator) is tapped. final VoidCallback? onJumpToPin; @@ -56,17 +50,6 @@ class ReaderActionBar extends StatelessWidget { return Row( mainAxisSize: MainAxisSize.min, children: [ - // Left — toggle the pinned state. - _ActionButton( - painter: PhosphorIcons.pushPin, - tooltip: hasPinned ? 'Unpin' : 'Pin', - enabled: onPin != null, - active: hasPinned, - onTap: onPin, - tokens: tokens, - ), - const SizedBox(width: 8), - // Right — the navigator. _ActionButton( painter: PhosphorIcons.caretLeft, tooltip: 'Back', @@ -107,6 +90,29 @@ class ReaderActionBar extends StatelessWidget { } } +/// The pin/unpin toggle — a single button that sits in the pane's +/// leading slot (before the title), separate from the navigator so the +/// two kinds of control don't read as one group (T-198). [pinned] +/// drives the accent + the Pin/Unpin label; [onTap] toggles. +class ReaderPinButton extends StatelessWidget { + const ReaderPinButton({super.key, required this.pinned, required this.onTap}); + + final bool pinned; + final VoidCallback? onTap; + + @override + Widget build(BuildContext context) { + return _ActionButton( + painter: PhosphorIcons.pushPin, + tooltip: pinned ? 'Unpin' : 'Pin', + enabled: onTap != null, + active: pinned, + onTap: onTap, + tokens: ClideTheme.of(context).surface, + ); + } +} + // --------------------------------------------------------------------------- // Private button widget // --------------------------------------------------------------------------- diff --git a/lib/builtin/tickets/src/ticket_detail_view.dart b/lib/builtin/tickets/src/ticket_detail_view.dart index b9874c1c..1d829972 100644 --- a/lib/builtin/tickets/src/ticket_detail_view.dart +++ b/lib/builtin/tickets/src/ticket_detail_view.dart @@ -73,6 +73,10 @@ class _TicketDetailViewState extends State { return ClidePaneChrome( title: d.id, subtitle: d.title, + leading: ReaderPinButton( + pinned: _nav?.hasPinned ?? false, + onTap: _onPin, + ), trailing: [ ReaderActionBar( canGoBack: _nav?.canGoBack ?? false, @@ -80,7 +84,6 @@ class _TicketDetailViewState extends State { hasPinned: _nav?.hasPinned ?? false, onBack: (_nav?.canGoBack ?? false) ? _onBack : null, onForward: (_nav?.canGoForward ?? false) ? _onForward : null, - onPin: _onPin, onJumpToPin: (_nav?.hasPinned ?? false) ? _onJumpToPin : null, onEdit: null, // tickets are pql records, not files ),