Skip to content

LongPressState enum tracks state of long presses#3113

Merged
raineorshine merged 5 commits intocybersemics:mainfrom
ethan-james:3072-bug-add-long-press-state-enum
Jul 7, 2025
Merged

LongPressState enum tracks state of long presses#3113
raineorshine merged 5 commits intocybersemics:mainfrom
ethan-james:3072-bug-add-long-press-state-enum

Conversation

@ethan-james
Copy link
Collaborator

Partial fix for #3072, #3073 and #3074

Add a longPress variable to the Redux store and a LongPressState enum to track the state of a long press. We need to keep shouldCancelGesture from allowing gestures in a variety of circumstances:

  1. while long pressing but before react-dnd has initiated a drag
  2. after scratching to cancel a drag

I tried to set it up as a proper state machine, which should provide some protection against state transitions coming in out of order. I haven't tested it at all yet.

@ethan-james ethan-james self-assigned this Jul 5, 2025
@ethan-james ethan-james marked this pull request as draft July 5, 2025 00:19
@raineorshine
Copy link
Contributor

raineorshine commented Jul 5, 2025

Thank you!

I tried to set it up as a proper state machine, which should provide some protection against state transitions coming in out of order. I haven't tested it at all yet.

That sounds fine, but let's console.error if an invalid state transition is attempted. I don't think we should rely on silently ignoring actions.

@ethan-james
Copy link
Collaborator Author

OK, I think I have fully replaced the functionality present in #3107 and fixed #3074 entirely. Should we discuss the possibility of replacing state.dragHold and state.dragInProgress with this new enum, or have we done enough already? 😄

I think that I could expand the Payload type for state.longPress like this:

interface InactivePayload {
  value: LongPressState.Inactive
}

interface DragHoldPayload {
  value: LongPressState.DragHold
  simplePath?: SimplePath
  sourceZone?: DragThoughtZone
}

interface DragInProgressPayload {
  value: LongPressState.DragInProgress
  // Sets state.draggingThoughts. Either hoveringPath or file must be set if value is true.
  draggingThoughts?: SimplePath[]
  hoveringPath?: Path
  hoverZone?: DropThoughtZone
  // Sets state.draggingFile. Either hoveringPath or file must be set if value is true.
  draggingFile?: boolean
  offset?: number
  sourceZone?: DragThoughtZone
}

interface DragCancelledPayload {
  value: LongPressState.DragCancelled
}

type Payload = InactivePayload | DragHoldPayload | DragInProgressPayload | DragCancelledPayload

@ethan-james ethan-james marked this pull request as ready for review July 7, 2025 17:15
Comment on lines +464 to +469
export enum LongPressState {
Inactive = 'Inactive',
DragHold = 'DragHold',
DragInProgress = 'DragInProgress',
DragCancelled = 'DragCancelled',
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I like to heavily document enums and types, since developers often go to them first in order to understand parts of the code.

Types are also declarative, so if a developer can understand the types enough to use them without having to understand the related algorithms or implementation details, that's a win.

diff --git a/src/@types/State.ts b/src/@types/State.ts
index 8278e02af8..68884214a6 100644
--- a/src/@types/State.ts
+++ b/src/@types/State.ts
@@ -121,7 +121,7 @@ interface State {
   /** The last undoable action that was executed. Usually this is the same as undoPatches.at(-1).actions[0]. However, on undo this will equal redoPatches.at(-1).actions[0]. This is important for special case animatons, like swapParent, that should be enabled not just when the action is originally executed, but also when it is reversed via undo. */
   lastUndoableActionType?: ActionType
   latestCommands: Command[]
-  /** A discrete state machine that limits available actions during long press. */
+  /** Tracks the state of long press and drag-and-drop. */
   longPress: LongPressState
   /** When a context is sorted, the manual sort order is saved so that it can be recovered when they cycle back through the sort options. If new thoughts have been added, their order relative to the original thoughts will be indeterminate, but both the old thoughts and the new thoughts will be sorted relative to themselves. The outer Index is keyed by parent ThoughtId, and the inner Index stores the manual ranks of each child at the time the context is sorted. This is stored in memory only and is lost when the app refreshes. */
   manualSortMap: Index<Index<number>>
diff --git a/src/constants.ts b/src/constants.ts
index 57aa05806b..5676bcb8ba 100644
--- a/src/constants.ts
+++ b/src/constants.ts
@@ -461,10 +461,15 @@ export enum AlertType {
   Undo = 'Undo',
 }
 
+/** A discrete state machine that tracks the state of long press and drag-and-drop. See the longPress reducer for valid transitions. */
 export enum LongPressState {
+  /** No long press or drag-and-drop action is in progress. */
   Inactive = 'Inactive',
+  /** The user has pressed a thought for longer than TIMEOUT_LONG_PRESS_THOUGHT without moving more than SCROLL_THRESHOLD px in order to activate the long press state.  */
   DragHold = 'DragHold',
+  /** The user is currently dragging a thought. */
   DragInProgress = 'DragInProgress',
+  /** The drag has been cancelled, but the user has not released their finger from the screen. */
   DragCancelled = 'DragCancelled',
 }

Copy link
Contributor

Choose a reason for hiding this comment

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

Added in 4f65fb9

if (!canceled && toggleMulticursorOnLongPress) {
dispatch(toggleMulticursor({ path: simplePath }))
if (!canceled) {
dispatch(longPress({ value: LongPressState.Inactive }))
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if this should be moved outside the conditional. Shouldn't state.longPress be set to inactive when the long press ends, regardless of whether the long press completed successfully or was canceled?

Copy link
Collaborator Author

@ethan-james ethan-james Jul 7, 2025

Choose a reason for hiding this comment

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

The onLongPressEnd function also runs when the pointer moves far enough to cancel a multicursor long press (DragHold becomes DragInProgress).

onLongPressEnd?.({ canceled: true })

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In that case, it does not run again when the long press actually ends.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see! Thanks for the clarification.

@raineorshine
Copy link
Contributor

Should we discuss the possibility of replacing state.dragHold and state.dragInProgress with this new enum, or have we done enough already? 😄

Yes, but separate PR once this is merged! Thanks!

@raineorshine raineorshine merged commit ed9edc0 into cybersemics:main Jul 7, 2025
3 checks passed
cindmichelle pushed a commit to cindmichelle/em that referenced this pull request Oct 21, 2025
Co-authored-by: Raine Revere <raine@cybersemics.org>
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