feat: Add shared Shift+Click range-selection engine and unify Table onto it (1/3)#95647
feat: Add shared Shift+Click range-selection engine and unify Table onto it (1/3)#95647TaduJR wants to merge 3 commits into
Conversation
|
@QichenZhu Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
Implementation: +368 / −69 (net +299) |
…shift-range-engine
|
There is a constraint. Reports/Expenses is built on the shared range engine in this PR (and knip fails the engine with no consumer), so part 1 would become engine + Reports/Expenses, which is roughly twice this PR's size. So lets continue here. WDYT? |
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |

Explanation of Change
This is part 1 of 3 for #90539, split out of #92155 per the review feedback on size. It adds the shared range-selection engine and moves Table onto it. No new surface gains shift+click here (Table already had its own implementation on main the other surfaces come in the follow-up PRs), but Table's existing behavior changes in four ways:
The
useShiftRangeSelectionhook is surface-agnostic: consumers tell it about plain clicks and bulk toggles, and on shift+click it emits a{toSelect, toDeselect}batch the consumer applies to its own selection store. It holds no selection state, just the session in a ref (the anchor plus the keys its ranges painted), touched only from event handlers. The anchor is the last row the user interacted with; if it disappears mid-session the hook falls back to the first selected row, then the first selectable one (this anchor model is the one flagged for internal-team confirmation on the original PR, unchanged here). The session tracks what it painted by key, so re-sorts and data refreshes can't misattribute a collapse, and a shrink only ever unchecks rows the shift-range itself checked, never rows the user picked individually. Disabled rows and group headers are excluded from ranges, anchors, and targets.libs/shiftRangeSelectionholds the shared event and batch helpers; selectability is decided once by the hook when it builds the batch, so appliers don't filter again. Two engine methods have no caller yet:seedRangeFromSelectionandclearAnchorare consumed by the Search provider in part 2.For Table, the middleware's own anchor refs and range walk are replaced by the shared hook, leaving one implementation for every Table-backed list (workspace Tags, Members, Cards, Categories, Distance rates, Per diem and so on).
SelectionButtongains a small shift+mousedown handler: preventDefault stops the browser's text selection from swallowing consecutive shift+clicks, with focus restored afterwards unless another element holds it.TransactionItemRow's checkbox forwards the Shift modifier as an optional argument; nothing reads it yet, existing callers are unaffected, and part 2 consumes it.Fixed Issues
$ #90539
PROPOSAL: #90539 (comment)
Tests
Scope note for QA: In this PR, Shift+click range selection only applies to the workspace table lists Categories, Tags, Members, Taxes, Distance rates, Per diem, Expensify Cards, and the Rules tabs (any workspace list with checkboxes). It is not yet implemented anywhere else the Reports tab (Expenses/Reports/Chats), the money request report view, report fields' List Values, and pickers like "Add existing expense" or "Share bank account" still toggle one row at a time. That is expected and not a bug: those surfaces ship in the two follow-up PRs. Shift+click is Web/Desktop only (there is no Shift key on mobile).
Verify: categories 2–6 are all checked.
Verify: 2–4 stay checked, 5–6 got unchecked.
Verify: 4–7 all become checked 2 and 3 stay checked.
Verify: 4–5 stay checked and 6–7 got unchecked, while 2 and 3 are still
checked.
Verify: 3–6 become checked while 2 stays unchecked (the range starts from
your last click row 3 so only that row comes back
Verify: 1–5 are checked.
Verify: only 1–3 remain checked.
Offline tests
Same as tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari