Skip to content

feat(context-menu): add right-click and long-press actions (#51)#91

Merged
Pairadux merged 2 commits intomainfrom
feat/context-menu-51
Mar 8, 2026
Merged

feat(context-menu): add right-click and long-press actions (#51)#91
Pairadux merged 2 commits intomainfrom
feat/context-menu-51

Conversation

@DevamPatel22
Copy link
Copy Markdown
Collaborator

Summary\n- add context menu actions on deck cards\n- add context menu actions on deck detail list items\n- support desktop right-click and mobile long-press entry points\n\nCloses #51

Copy link
Copy Markdown
Owner

@Pairadux Pairadux left a comment

Choose a reason for hiding this comment

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

A few things to address:

Duplicated menu construction - _showContextMenuAt() in DeckDetailScreen and _showContextMenu() in DeckCard build identical popup menu items separately. Define them once so adding new actions later only requires one change.

Inconsistent pattern - Deck context menus: DeckCard shows the menu, then fires onContextMenuAction. Card context menus: DeckDetailScreen shows the menu and handles the action directly. Pick one approach for both.

Gesture conflicts - Wrapping InkWell inside GestureDetector can cause issues (ink splash may fire before the context menu appears). Consider using a single gesture layer instead.

onLongPress silently dropped - When onContextMenuAction is set, the onLongPress callback gets nulled out. Not a bug right now, but could surprise future callers.

Missing from DeckListScreen - Right-click and long-press context menus only work inside a deck (on DeckDetailScreen). They don't work on the main deck list screen. If that's intentional for this PR, please open a follow-up issue to add it there too.

Good work overall. Clean enum, proper mounted checks, and the disabled "Move" placeholder is a nice touch.

@Pairadux
Copy link
Copy Markdown
Owner

Pairadux commented Mar 3, 2026

If adding to core is needed for first thing, let me know, but its probably not a problem

DevamPatel22 and others added 2 commits March 8, 2026 15:11
Move context menu enum, items, and gesture handling into a shared
ContextMenuRegion widget. This eliminates duplicated menu-showing
logic across DeckCard and DeckDetailScreen.
@Pairadux Pairadux force-pushed the feat/context-menu-51 branch from 90d67b3 to 09cdd2d Compare March 8, 2026 19:13
@Pairadux Pairadux merged commit 31f2a2a into main Mar 8, 2026
@Pairadux Pairadux deleted the feat/context-menu-51 branch March 8, 2026 19:13
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.

2 participants