-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
List View: Add multi-select behaviour for blocks when shift key is selected #38314
Changes from all commits
8749179
4e75817
cfba501
51376c5
d2fd70d
b6bb2db
06c731c
edcff8b
1362dba
74602ca
62f32e4
8fbb905
999568e
d884e0e
27d63bc
c5853bc
6cb4fb3
38b0e5b
025087f
c3b6a1e
b321851
db31ffd
4f0e0bb
5511896
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -48,6 +48,7 @@ function ListViewBlock( { | |
showBlockMovers, | ||
path, | ||
isExpanded, | ||
selectedClientIds, | ||
} ) { | ||
const cellRef = useRef( null ); | ||
const [ isHovered, setIsHovered ] = useState( false ); | ||
|
@@ -104,14 +105,22 @@ function ListViewBlock( { | |
|
||
const selectEditorBlock = useCallback( | ||
( event ) => { | ||
event.stopPropagation(); | ||
selectBlock( clientId ); | ||
selectBlock( event, clientId ); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the background behind this change? Just an FYI, there was a bug that someone noticed yesterday that's related to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the heads-up about that PR! I'll rebase this PR tomorrow and do some extra testing to see if I need to refactor a little more there to preserve that fix. One of the main changes in this PR is that we need to pass up the click and keyboard events to where we use the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've adjusted the logic slightly (added in a separate |
||
}, | ||
[ clientId, selectBlock ] | ||
); | ||
|
||
const selectDuplicatedBlock = useCallback( | ||
( newClientId ) => { | ||
selectBlock( undefined, newClientId ); | ||
}, | ||
[ selectBlock ] | ||
); | ||
|
||
const toggleExpanded = useCallback( | ||
( event ) => { | ||
// Prevent shift+click from opening link in a new window when toggling. | ||
event.preventDefault(); | ||
event.stopPropagation(); | ||
if ( isExpanded === true ) { | ||
collapse( clientId ); | ||
|
@@ -154,6 +163,14 @@ function ListViewBlock( { | |
) | ||
: __( 'Options' ); | ||
|
||
// Only include all selected blocks if the currently clicked on block | ||
// is one of the selected blocks. This ensures that if a user attempts | ||
// to alter a block that isn't part of the selection, they're still able | ||
// to do so. | ||
const dropdownClientIds = selectedClientIds.includes( clientId ) | ||
? selectedClientIds | ||
: [ clientId ]; | ||
|
||
return ( | ||
<ListViewLeaf | ||
className={ classes } | ||
|
@@ -188,6 +205,7 @@ function ListViewBlock( { | |
tabIndex={ tabIndex } | ||
onFocus={ onFocus } | ||
isExpanded={ isExpanded } | ||
selectedClientIds={ selectedClientIds } | ||
/> | ||
</div> | ||
) } | ||
|
@@ -228,7 +246,7 @@ function ListViewBlock( { | |
<TreeGridCell className={ listViewBlockSettingsClassName }> | ||
{ ( { ref, tabIndex, onFocus } ) => ( | ||
<BlockSettingsDropdown | ||
clientIds={ [ clientId ] } | ||
clientIds={ dropdownClientIds } | ||
icon={ moreVertical } | ||
label={ settingsAriaLabel } | ||
toggleProps={ { | ||
|
@@ -238,7 +256,7 @@ function ListViewBlock( { | |
onFocus, | ||
} } | ||
disableOpenOnArrowDown | ||
__experimentalSelectBlock={ selectBlock } | ||
__experimentalSelectBlock={ selectDuplicatedBlock } | ||
/> | ||
) } | ||
</TreeGridCell> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { getCommonDepthClientIds } from '../utils'; | ||
|
||
describe( 'getCommonDepthClientIds', () => { | ||
it( 'should return start and end when no depth is provided', () => { | ||
const result = getCommonDepthClientIds( | ||
'start-id', | ||
'clicked-id', | ||
[], | ||
[] | ||
); | ||
|
||
expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); | ||
} ); | ||
|
||
it( 'should return deepest start and end when depths match', () => { | ||
const result = getCommonDepthClientIds( | ||
'start-id', | ||
'clicked-id', | ||
[ 'start-1', 'start-2', 'start-3' ], | ||
[ 'end-1', 'end-2', 'end-3' ] | ||
); | ||
|
||
expect( result ).toEqual( { start: 'start-id', end: 'clicked-id' } ); | ||
} ); | ||
|
||
it( 'should return shallower ids when start is shallower', () => { | ||
const result = getCommonDepthClientIds( | ||
'start-id', | ||
'clicked-id', | ||
[ 'start-1' ], | ||
[ 'end-1', 'end-2', 'end-3' ] | ||
); | ||
|
||
expect( result ).toEqual( { start: 'start-id', end: 'end-2' } ); | ||
} ); | ||
|
||
it( 'should return shallower ids when end is shallower', () => { | ||
const result = getCommonDepthClientIds( | ||
'start-id', | ||
'clicked-id', | ||
[ 'start-1', 'start-2', 'start-3' ], | ||
[ 'end-1', 'end-2' ] | ||
); | ||
|
||
expect( result ).toEqual( { start: 'start-3', end: 'clicked-id' } ); | ||
} ); | ||
} ); |
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.
As of #33320 the
rootClientId
is no longer used in this function, so I think it can safely be removed here.