feat(bulk-editor): shift+click range selection and animated notices banner - #23511
Open
vraja-pro wants to merge 3 commits into
Open
feat(bulk-editor): shift+click range selection and animated notices banner#23511vraja-pro wants to merge 3 commits into
vraja-pro wants to merge 3 commits into
Conversation
…anner - selection.js: add selectRange reducer that merges a contiguous id range (anchorId → targetId within the editable id list) into the current selection - bulk-editor-content.js: track anchorId ref; wrap toggleRow so plain clicks set the anchor and shift+clicks dispatch selectRange; clear anchor on selectAll/deselectAll - table-row.js: pass event.shiftKey as second arg to onToggleRow so the checkbox onChange event reaches the range-selection logic - bulk-action-bar.js: wrap BulkActionsNotices in AnimateHeight driven by useSlotFills, so the Premium notice banner slides in/out smoothly instead of causing a layout jump Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Coverage Report for CI Build 0Coverage increased (+0.001%) to 55.56%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats💛 - Coveralls |
…tion onChange on a checkbox does not carry modifier-key info, so reading event.shiftKey there is unreliable (always undefined on Mac/Chrome). Replace the event-argument approach with document keydown/keyup listeners that keep isShiftDownRef in sync; onToggleRow reads that ref instead. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR improves the Bulk Editor UX by adding shift+click range selection for row checkboxes and animating the Bulk Actions notices area so banners appear/disappear without layout jumps.
Changes:
- Add a
selectRangeaction to merge a contiguous id range into the existing selection. - Track an “anchor” row in
BulkEditorContentto support shift+click range selection, and reset the anchor on select-all/deselect-all. - Wrap the bulk notices region in
AnimateHeight, expanding/collapsing based on whether notices are present.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/js/src/bulk-editor/store/selection.js | Adds selectRange reducer to support shift+click range selection. |
| packages/js/src/bulk-editor/components/bulk-editor-content.js | Implements anchor-based range selection and updates select-all/deselect-all handling. |
| packages/js/src/bulk-editor/components/bulk-action-bar.js | Animates notices/banner region via AnimateHeight and slot-fill detection. |
Comments suppressed due to low confidence (2)
packages/js/src/bulk-editor/components/bulk-editor-content.js:153
- The Shift key tracking can get stuck as
trueif the window loses focus while Shift is held (nokeyupfires), which would make subsequent plain clicks behave like shift+click. Consider resettingisShiftDownRefonwindow.blur(and/orvisibilitychange) to keep the modifier state consistent.
document.addEventListener( "keydown", onDown );
document.addEventListener( "keyup", onUp );
return () => {
document.removeEventListener( "keydown", onDown );
document.removeEventListener( "keyup", onUp );
packages/js/src/bulk-editor/components/bulk-editor-content.js:161
- Shift+click range selection is introduced here, but there are no unit tests covering the anchor + range behavior (including anchor reset on selectAll/deselectAll). Since
bulk-editor-content.test.jsalready exists, adding a focused test for dispatchingselectRangeon shift+click would protect this UX behavior.
const onToggleRow = useCallback( ( id ) => {
if ( isShiftDownRef.current && anchorIdRef.current !== null ) {
const allEditableIds = items.filter( ( item ) => item.editable ).map( ( item ) => item.id );
selectRange( { anchorId: anchorIdRef.current, targetId: id, allIds: allEditableIds } );
} else {
Comment on lines
+135
to
+137
| // Tracks whether the Shift key is currently held, via document events — onChange on a checkbox | ||
| // doesn't carry modifier-key info, so we can't rely on event.shiftKey there. | ||
| const isShiftDownRef = useRef( false ); |
Comment on lines
+28
to
+33
| /** | ||
| * Merges a contiguous range of ids (from anchorId to targetId within allIds) into the current selection. | ||
| */ | ||
| selectRange: ( state, { payload: { anchorId, targetId, allIds } } ) => { | ||
| const anchorIndex = allIds.indexOf( anchorId ); | ||
| const targetIndex = allIds.indexOf( targetId ); |
- Reset isShiftDownRef on window blur so holding Shift while switching windows (Alt+Tab) doesn't leave the ref stuck, causing the next plain click to behave as a shift+click. - Add unit tests for the selectRange reducer: forward range, reverse range, merge with existing selection, and no-op on unknown anchor/target. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Context
Three UX improvements for the bulk editor selection and AI notice flow:
selectRangeaction and the shiftKey plumbing that the cleanup depends on.)Summary
This PR can be summarized in the following changelog entry:
Relevant technical choices:
selectRangereducer added toselection.js— merges the contiguous id range (anchorId → targetId within the ordered editable-item list) into the existing selection without replacing it.BulkEditorContenttracksanchorIdRefand wrapstoggleRow: a plain click updates the anchor; a shift+click dispatchesselectRange. The anchor is cleared onselectAll/deselectAll.handleToggleinBulkEditorRowpassesevent.shiftKeyas a second arg so the shift state flows through the row-toggle seam without coupling the row to the range logic.BulkActionsNoticesis wrapped inAnimateHeightdriven byuseSlotFills(BULK_NOTICES_SLOT): the notices section expands/collapses smoothly when Premium mounts or unmounts its fill.Test instructions
Test instructions for the acceptance test before the PR gets merged
This PR can be acceptance tested by following these steps:
Relevant test scenarios
Test instructions for QA when the code is in the RC
QA can test this PR by following these steps:
Impact check
This PR affects the following parts of the plugin, which may require extra testing:
selection.js,bulk-editor-content.js,table-row.js)bulk-action-bar.js)Other environments
[shopify-seo], added test instructions for Shopify and attached theShopifylabel to this PR.[yoast-doc-extension], added test instructions for Yoast SEO for Google Docs and attached theGoogle Docs Add-onlabel to this PR.Documentation
Quality assurance
grunt build:imagesand commited the results, if my PR introduces new images or SVGs.Innovation
innovationlabel.Related: https://github.com/Yoast/plugins-automated-testing/issues/3075