-
Notifications
You must be signed in to change notification settings - Fork 8
feat(ligretto): unify card interactions and drag and drop #686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
memebattle-dev
wants to merge
18
commits into
master
Choose a base branch
from
docs/issue-685-drag-drop-plan
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 16 commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
aeac54a
docs(ligretto): plan card drag and drop
memebattle-dev 150796e
feat(ligretto): add card drag and drop
memebattle-dev 662cf6f
fix(ligretto): exclude tests from shared build
memebattle-dev b488870
refactor(ligretto): address drag and drop review
memebattle-dev fbfab3c
refactor(ligretto): simplify card placement dnd
memebattle-dev 6904535
fix(ligretto): [verified] address drag and drop review
memebattle-dev 7b90271
refactor(ligretto): unify card interaction state
memebattle-dev dc0b9f7
fix(ligretto): [verified] target onboarding drop surfaces
memebattle-dev 2ef5127
test(ligretto): [verified] place onboarding cards manually
memebattle-dev b8e50f8
refactor(ligretto): [verified] address card interaction review
memebattle-dev c3dce9d
refactor(ligretto): refine card interaction hooks
memebattle-dev ba5cfaa
[verified] refactor(ligretto): address card interaction review
memebattle-dev 4743ab1
[verified] refactor(ligretto): keep card interaction mechanics in hooks
memebattle-dev 32ddcbc
fix(ligretto): cancel native drag before card hotkeys
memebattle-dev 7048d9b
fix(ligretto): ignore cancelled touch compatibility clicks
memebattle-dev e104ef0
refactor(ligretto): remove custom drag cancellation
memebattle-dev a1554e3
refactor(ligretto): [verified] address card interaction backlog
memebattle-dev 3b5d4f4
fix(ligretto): [verified] preserve touch taps and unify cleanup
memebattle-dev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
16 changes: 0 additions & 16 deletions
16
apps/ligretto-frontend/src/features/cardFocus/ui/CardFocusContext.ts
This file was deleted.
Oops, something went wrong.
64 changes: 0 additions & 64 deletions
64
apps/ligretto-frontend/src/features/cardFocus/ui/CardFocusProvider.tsx
This file was deleted.
Oops, something went wrong.
48 changes: 0 additions & 48 deletions
48
apps/ligretto-frontend/src/features/cardFocus/ui/useCardFocus.ts
This file was deleted.
Oops, something went wrong.
54 changes: 54 additions & 0 deletions
54
apps/ligretto-frontend/src/features/cardInteraction/README.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| # Card interaction ownership | ||
|
|
||
| `CardInteractionProvider` owns an explicit `idle | focused | dragging` reducer. | ||
| It uses standard dnd-kit mouse/touch sensors, outside-click dismissal and | ||
| central provider enablement. It does not read Redux, select cards, validate game | ||
| rules, dispatch game commands, or render a card. | ||
|
|
||
| Components opt into behavior through hooks: | ||
|
|
||
| - `useCardInteraction(target, identityDependencies)` supplies selection flags and | ||
| pointer toggling. Identity changes and unmount clear only that target. The | ||
| argument-free overload exposes the current target and explicit clearing. | ||
| - `useDraggableCard(target, card, disabled?)` attaches the native source to the | ||
| component's own element. Card/target identity changes, disablement and unmount | ||
| invalidate selection; components decide how to hide the original while dragging. | ||
| - `useDroppableTarget(target, onDrop)` attaches a semantic destination and invokes | ||
| its owner through dnd data only for a live, enabled source and destination. The owner validates current game rules and dispatches its own command. | ||
| - `useCardHotkey(key, callback)` clears selection and invokes the owner when the | ||
| provider is enabled, without changing the native gesture. | ||
| It has no options object. Clearing then toggling deliberately keeps a repeated | ||
| row/open-stack shortcut selected; pointer activation still toggles it off. | ||
| - `useCardDragTarget()` derives a live drag target from dnd-kit and its registered | ||
| source, independently of selection. `PlayerCardDragOverlay` uses it to look up current Redux data and | ||
| render the dnd-kit presentation primitive and game card outside this feature. | ||
|
|
||
| The target union and `getInteractionTargetKey` cover row, open-stack and playground | ||
| identities. Hooks return behavior/data, never styles or card wrapper elements. | ||
|
|
||
| ## Gesture and command semantics | ||
|
|
||
| Gameplay shortcuts remain available during pending and active mouse/touch gestures. | ||
| They do not cancel native dragging or prevent a valid drop on release. Source | ||
| visibility and the overlay follow the native gesture, not hotkey selection. | ||
| There are no custom sensor adapters, Escape interception or release-click guards. | ||
| The standard dnd-kit `onDragCancel` lifecycle only synchronizes reducer state. | ||
| Disabled interactions and invalid or stale sources/destinations cannot dispatch drops. | ||
|
|
||
| Click placement deliberately keeps its selected source until a server-confirmed | ||
| card identity update. It continues to submit the explicit destination to backend | ||
| validation, whereas drag owners prevalidate against their current destination. | ||
| Do not clear click selection optimistically or auto-place value-1 source cards. | ||
|
|
||
| Onboarding keeps its dedicated playground, callback routing and outline refs. | ||
| Its source owners include step availability in their identity dependencies so a | ||
| selection cannot outlive the step that enabled it. | ||
|
|
||
| ## Tests | ||
|
|
||
| Feature tests drive real dnd-kit mouse and touch events without a Redux provider; component | ||
| tests verify command payloads, overlay/source visibility, shortcut availability, | ||
| repeated selection, onboarding refs, stale/disabled drops and hotkeys during native gestures. The | ||
| native mouse sensor suppresses document clicks for 50 ms after release; tests that | ||
| mix drag and click cases must allow that teardown to finish. No new E2E harness is | ||
| required by this refinement. |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please verify before fixing:
onMouseDown→onClickis an unacknowledged trade-off, and with the 6 px MouseSensor constraint a jittery press is swallowed.onMouseDown={onClick}was introduced deliberately in 8c6e400 (LIG-144, "put card on mousedown instead of click") as a speed-game affordance. This PR flips everyCardtoonClickwithout mentioning it in the description or README.For draggable cards the combination with
activationConstraint: { distance: 6 }has a concrete failure: mousedown on a row card, drift ≥ 6 px, mouseup over the card itself → dnd-kit activates,over === null,onDragEndonly dispatchesdragTerminal, and the 50 ms capture-phase click suppression preventsonCardActivate. The card is neither selected nor placed; on the base branch it toggled on mousedown regardless of drift. For non-draggable cards (stack deck, ligretto pack, onboarding) activation now waits for mouseup and press-drag-off-release no longer fires.If the change is intended, please say so in the PR body and the README. If not, options are a larger distance, or activating on
onMouseDownfor non-draggable cards only.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3b5d4f4 documents the current release-time activation, non-draggable latency and threshold-crossing cancelled placement in the README; the PR description is synchronized. LIG-144 deliberately used mousedown. Should we retain the current onClick trade-off, or restore mousedown activation for non-draggable cards? I left behavior unchanged rather than choose silently.