Temporary strategies UI/UX#41
Temporary strategies UI/UX#41SunkenInTime wants to merge 1 commit intocursor/temporary-strategy-ux-19d9from
Conversation
- Add amber (Draft Copy) and cyan (Quick Board) accent color tokens to Settings - Create TemporarySessionBar widget: full-width colored bar between header and map showing session state, description, and contextual action buttons (Save to Original / Save as New / Save Board / Discard) - Redesign StrategyQuickSwitcher: show original name with glowing colored dot indicator and tinted border in temp mode; replace 'Temporary Copy' button with clearer 'Draft Copy' button with pen icon and amber hover state; remove ambiguous 'Finish' button (actions now in session bar) - Redesign temporary session dialogs with Lucide icons, color-coded action buttons, and clearer copy (e.g. 'Save Draft Changes?' with pen icon) - Fix Quick Board double-load bug: navigateWithLoading was re-calling loadFromHive with default saved sessionKind, overwriting the quickBoard state set by createQuickBoard; added skipLoad parameter - Update Quick Board button in folder navigator with cyan zap icon - Remove unused strategy_provider import from strategy_save_icon_button Co-authored-by: Dara Adedeji <SunkenInTime@users.noreply.github.com>
|
Cursor Agent can help with this pull request. Just |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR redesigns the temporary strategy UI/UX by introducing a new Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User in StrategyView] --> B{isTemporarySession?}
B -- No --> C[TemporarySessionBar hidden\nDraftCopyButton visible]
B -- Yes --> D[TemporarySessionBar shown\nDraftCopyButton hidden]
C -->|Click Draft Copy| E[startTemporaryCopyFromCurrentStrategy\nsessionKind = temporaryCopy]
C -->|Click Quick Board| F[createQuickBoard\nsessionKind = quickBoard\nskipLoad = true]
E --> D
F --> D
D -->|Draft Mode: Save to Original| G[overwriteOriginalFromTemporaryCopy]
D -->|Draft Mode: Save as New| H[showStrategySaveDetailsDialog\n→ saveTemporarySessionAsNewStrategy]
D -->|Quick Board: Save Board| H
D -->|Click X / Discard| I[_showExitConfirmation dialog]
I -->|Confirmed| J[discardTemporarySession\nsessionKind = saved]
I -->|Cancelled| D
G --> K[sessionKind = saved\nTemporarySessionBar animates out]
H --> K
J --> K
Last reviewed commit: 0401af2 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| Future<void> _exitDraft() async { | ||
| final strategy = ref.read(strategyProvider); | ||
| final confirmed = await _showExitConfirmation( | ||
| isQuickBoard: strategy.isQuickBoard, | ||
| ); | ||
| if (confirmed) { | ||
| await ref.read(strategyProvider.notifier).discardTemporarySession(); | ||
| } |
There was a problem hiding this comment.
ref.read called after async gap without mounted check
ref.read(strategyProvider.notifier) is accessed after the await _showExitConfirmation(...) async gap with no mounted guard. In Riverpod's ConsumerStatefulWidget, accessing ref after the widget has been disposed throws a StateError. If the user dismisses the parent screen while the confirmation dialog is still open, ref will be dead when the dialog eventually resolves.
| Future<void> _exitDraft() async { | |
| final strategy = ref.read(strategyProvider); | |
| final confirmed = await _showExitConfirmation( | |
| isQuickBoard: strategy.isQuickBoard, | |
| ); | |
| if (confirmed) { | |
| await ref.read(strategyProvider.notifier).discardTemporarySession(); | |
| } | |
| Future<void> _exitDraft() async { | |
| final strategy = ref.read(strategyProvider); | |
| final confirmed = await _showExitConfirmation( | |
| isQuickBoard: strategy.isQuickBoard, | |
| ); | |
| if (!mounted) return; | |
| if (confirmed) { | |
| await ref.read(strategyProvider.notifier).discardTemporarySession(); | |
| } | |
| } |
| decoration: BoxDecoration( | ||
| color: accentColor, | ||
| shape: BoxShape.circle, | ||
| boxShadow: [ | ||
| BoxShadow( | ||
| color: accentColor!.withValues(alpha: 0.5), | ||
| blurRadius: 4, | ||
| ), | ||
| ], | ||
| ), | ||
| ), | ||
| ], |
There was a problem hiding this comment.
Fragile null assertion on accentColor
accentColor is typed Color? and the null assertion operator ! is used on line 397 (accentColor!.withValues(alpha: 0.5)). This is safe today because StrategySessionKind only has quickBoard and temporaryCopy as non-saved values (covering all isTemporarySession cases), so _sessionAccentColor is always non-null here.
However, if a third StrategySessionKind were ever added, this would throw a Null check operator used on a null value crash at runtime. The type system cannot enforce the invariant. Consider using a local non-null binding or a fallback color instead:
if (currentStrategy.isTemporarySession) ...[
const SizedBox(width: 10),
Container(
width: 7,
height: 7,
decoration: BoxDecoration(
color: accentColor,
shape: BoxShape.circle,
boxShadow: [
BoxShadow(
color: (accentColor ?? Colors.white).withValues(alpha: 0.5),
blurRadius: 4,
),
],
),
),
],| if (strategy.isTemporaryCopy) ...[ | ||
| _SessionBarButton( | ||
| onPressed: _saveToOriginal, | ||
| icon: LucideIcons.save, | ||
| label: 'Save to Original', | ||
| accentColor: accentColor, | ||
| isPrimary: true, | ||
| ), | ||
| const SizedBox(width: 6), | ||
| _SessionBarButton( | ||
| onPressed: _saveAsNew, | ||
| icon: LucideIcons.filePlus, | ||
| label: 'Save as New', | ||
| accentColor: accentColor, | ||
| isPrimary: false, | ||
| ), | ||
| ] else ...[ | ||
| _SessionBarButton( | ||
| onPressed: _saveAsNew, | ||
| icon: LucideIcons.save, | ||
| label: 'Save Board', | ||
| accentColor: accentColor, | ||
| isPrimary: true, | ||
| ), | ||
| ], |
There was a problem hiding this comment.
Session bar action buttons lack a loading/disabled state
_SessionBarButton.onPressed is typed as a non-nullable VoidCallback, and the button always shows a click cursor. If a user rapidly clicks "Save to Original" or "Save Board" before the async operation completes, _saveToOriginal / _saveAsNew will be invoked concurrently, potentially triggering duplicate saves or writes to Hive.
Consider adding an isLoading / busy flag to _TemporarySessionBarState and passing a nullable onPressed to _SessionBarButton (similar to how _DraftCopyButton already handles its VoidCallback?) to disable the button while the operation is in flight.
2 participants
Redesign the UI/UX for temporary strategies (Draft Copy and Quick Board) to improve intuitiveness, clarity, and visual consistency.
The previous implementation felt unintuitive, with vague state indicators (e.g., "temporary strategy" replacing the name) and inconsistent design. This PR introduces clear visual cues (color-coded session bars, glowing dots, tinted borders), meaningful Lucide icons, and redesigned dialogs to make the temporary strategy state explicit and the user flow smoother. It also fixes a pre-existing bug where the Quick Board state was incorrectly overwritten after creation.