Repository navigation
Mobile: fix the playlist reorder crash when the list changes mid-drag (LUMEN-4) #83
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
Merged
Merged
Changes from all commits
Commits
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| import { useCallback, useEffect, useLayoutEffect, useRef, useState } from "react"; | ||
| import ReorderableList, { | ||
| type ReorderableListProps, | ||
| type ReorderableListReorderEvent, | ||
| } from "react-native-reorderable-list"; | ||
| import { scheduleOnRN } from "react-native-worklets"; | ||
| import { rebaseDrop } from "../lib/reorder-drop"; | ||
|
|
||
| /** The library's default `animationDuration`: how long a dropped row takes to settle. */ | ||
| const DROP_ANIMATION_MS = 200; | ||
| /** Slack after the drop animation for the library's onReorder to reach JS. */ | ||
| export const LET_GO_AFTER_DROP_MS = 300; | ||
|
|
||
| type SteadyReorderableListProps<T> = Omit< | ||
| ReorderableListProps<T>, | ||
| "keyExtractor" | "onDragStart" | "onDragEnd" | ||
| > & { | ||
| keyExtractor: (item: T, index: number) => string; | ||
| }; | ||
|
|
||
| /** | ||
| * A ReorderableList whose rows hold still from the start of a drag until its | ||
| * drop is handled. | ||
| * | ||
| * The library reports a drop as indices into the rows it showed when the | ||
| * drag began, and before that reads `keyExtractor(data[i])` for each index | ||
| * in between without checking it against the current data. New data | ||
| * mid-drag (a refetch) then moved the wrong row, and a shorter list crashed | ||
| * the app (Sentry LUMEN-4). So the list keeps the rows it started the drag | ||
| * on, and `onReorder` gets the drop replayed onto the latest `data`, which | ||
| * the list shows from then on. A drop whose row is gone reports nothing. | ||
| */ | ||
| export function SteadyReorderableList<T>({ | ||
| data, | ||
| keyExtractor, | ||
| onReorder, | ||
| ...props | ||
| }: SteadyReorderableListProps<T>) { | ||
| const [held, setHeld] = useState<T[] | null>(null); | ||
| const shown = held ?? data; | ||
| const letGoMs = (props.animationDuration ?? DROP_ANIMATION_MS) + LET_GO_AFTER_DROP_MS; | ||
| // The drag callbacks run from the UI thread, and the drop from the closure | ||
| // the list captured when the finger lifted, so they read the latest render | ||
| // from here. | ||
| const latest = useRef({ shown, data, keyExtractor, onReorder, letGoMs }); | ||
| useLayoutEffect(() => { | ||
| latest.current = { shown, data, keyExtractor, onReorder, letGoMs }; | ||
| }); | ||
| // The rows the current drag's indices refer to, kept even if the hold | ||
| // lets go before its drop arrives. | ||
| const dragRows = useRef<T[] | null>(null); | ||
| const letGoTimer = useRef<ReturnType<typeof setTimeout>>(undefined); | ||
| useEffect(() => () => clearTimeout(letGoTimer.current), []); | ||
|
|
||
| const hold = useCallback(() => { | ||
| clearTimeout(letGoTimer.current); | ||
| dragRows.current = latest.current.shown; | ||
| setHeld(dragRows.current); | ||
| }, []); | ||
| // The library calls onReorder once the dropped row has settled, except for | ||
| // a row put back where it was, or a finger lifted exactly on the new slot. | ||
| // Let go a little after the drop animation unless the drop came first. | ||
| const settle = useCallback(() => { | ||
| clearTimeout(letGoTimer.current); | ||
| letGoTimer.current = setTimeout(() => setHeld(null), latest.current.letGoMs); | ||
| }, []); | ||
| const onDragStart = useCallback(() => { | ||
| "worklet"; | ||
| scheduleOnRN(hold); | ||
| }, [hold]); | ||
| const onDragEnd = useCallback(() => { | ||
| "worklet"; | ||
| scheduleOnRN(settle); | ||
| }, [settle]); | ||
| const handleReorder = useCallback(({ from, to }: ReorderableListReorderEvent) => { | ||
| clearTimeout(letGoTimer.current); | ||
| const { shown: dragged, data: current, keyExtractor: key, onReorder: report } = | ||
| latest.current; | ||
| const rows = dragRows.current ?? dragged; | ||
| dragRows.current = null; | ||
| setHeld(null); | ||
| const move = rebaseDrop(rows.map(key), current.map(key), from, to); | ||
| if (move) report(move); | ||
| }, []); | ||
|
|
||
| return ( | ||
| <ReorderableList<T> | ||
| {...props} | ||
| data={shown} | ||
| keyExtractor={keyExtractor} | ||
| onReorder={handleReorder} | ||
| onDragStart={onDragStart} | ||
| onDragEnd={onDragEnd} | ||
| /> | ||
| ); | ||
| } |
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,83 @@ | ||
| import { movePlaylistEntry } from "@music-library/core"; | ||
|
|
||
| /** | ||
| * Drops from react-native-reorderable-list, checked against the rows a list | ||
| * holds when they land. | ||
| * | ||
| * The list reports a drop as indices into the rows it showed when the drag | ||
| * began, one drop animation after the finger lifts. A refetch can replace | ||
| * those rows in the meantime, so the indices may point at other rows, or past | ||
| * the end of a shorter list. | ||
| */ | ||
|
|
||
| export type ReorderMove = { from: number; to: number }; | ||
|
|
||
| /** Whether `index` is a slot in `list`. */ | ||
| export function isIndexIn(list: readonly unknown[], index: number): boolean { | ||
| return Number.isInteger(index) && index >= 0 && index < list.length; | ||
| } | ||
|
|
||
| /** | ||
| * A list key for a row that may be missing. The list library reads keys by | ||
| * index, and a stale index past the end of the data hands it `undefined`. | ||
| */ | ||
| export function rowKeyOrIndex(row: { key: string } | undefined, index: number): string { | ||
| return row?.key ?? String(index); | ||
| } | ||
|
|
||
| /** | ||
| * `keys` with each repeat numbered in list order ("k", "k#1", "k#2"), so rows | ||
| * that share a key still get distinct ones. | ||
| */ | ||
| export function numberRepeatedKeys(keys: readonly string[]): string[] { | ||
| const seen = new Map<string, number>(); | ||
| return keys.map((key) => { | ||
| const count = seen.get(key) ?? 0; | ||
| seen.set(key, count + 1); | ||
| return count === 0 ? key : `${key}#${count}`; | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * The move a drop makes on `current`, where `from` and `to` index `dragged` | ||
| * (the row keys the drag began on). The dragged row goes under the row it | ||
| * was dropped under; when that row is gone, or the drop was at the top, it | ||
| * goes above the row it was dropped above. Null when there's nothing to | ||
| * move: an index outside `dragged`, a dragged row that's gone, no neighbour | ||
| * left to place it by, or a row already where the drop puts it. Keys must | ||
| * be unique (see `numberRepeatedKeys`). | ||
| */ | ||
| export function rebaseDrop( | ||
| dragged: readonly string[], | ||
| current: readonly string[], | ||
| from: number, | ||
| to: number, | ||
| ): ReorderMove | null { | ||
| if (from === to || !isIndexIn(dragged, from) || !isIndexIn(dragged, to)) return null; | ||
| const fromNow = current.indexOf(dragged[from]); | ||
| if (fromNow < 0) return null; | ||
| // Both lists without the dragged row: the drop puts it between | ||
| // others[to - 1] and others[to], and the move's `to` indexes `rest` the | ||
| // same way (out first, then back in). | ||
| const others = dragged.filter((_, i) => i !== from); | ||
| const rest = current.filter((_, i) => i !== fromNow); | ||
| const below = to > 0 ? rest.indexOf(others[to - 1]) : -1; | ||
| const above = below < 0 && to < others.length ? rest.indexOf(others[to]) : -1; | ||
| const toNow = below >= 0 ? below + 1 : above; | ||
| if (toNow < 0 || toNow === fromNow) return null; | ||
| return { from: fromNow, to: toNow }; | ||
| } | ||
|
|
||
| /** | ||
| * The playlist after moving the row at `from` to `to`, or null unless both | ||
| * are distinct slots in `entries`: a reorder built from an index outside the | ||
| * list would send the server an undefined track id. | ||
| */ | ||
| export function playlistAfterDrop<T extends { position: number }>( | ||
| entries: readonly T[], | ||
| from: number, | ||
| to: number, | ||
| ): T[] | null { | ||
| if (from === to || !isIndexIn(entries, from) || !isIndexIn(entries, to)) return null; | ||
| return movePlaylistEntry(entries, from, to); | ||
| } |
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.