Skip to content
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

Feat : double-click to open/close block in List View #64641

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

nirav7707
Copy link
Contributor

What?

resolve #64625

Why?

To enhance accessibility

How?

Registering the doubleClick event listener to ListViewBlock component

component ListViewBlock to expand and collapse
@nirav7707 nirav7707 requested a review from ellatrix as a code owner August 20, 2024 11:27
Copy link

github-actions bot commented Aug 20, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: nirav7707 <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: jasmussen <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: richtabor <[email protected]>
Co-authored-by: paaljoachim <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: Drivingralle <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Feature] List View Menu item in the top toolbar to select blocks from a list of links. labels Aug 20, 2024
@Mamaduka Mamaduka requested a review from andrewserong August 21, 2024 08:35
@andrewserong andrewserong added the Needs Design Feedback Needs general design feedback. label Aug 21, 2024
Copy link
Contributor

@andrewserong andrewserong left a comment

Choose a reason for hiding this comment

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

Thanks for putting up a PR @nirav7707! For folks who might not be familiar with the feature it can be helpful to add some testing instructions or a screengrab to the PR description to help anyone looking at the PR to know what to look for or test. Even a simple PR can sometimes be a bit tricky to review given the complexity of Gutenberg.

In practice this is testing well for me in the editor, here's a quick video:

2024-08-22.09.58.17.mp4

I've added the "Needs Design Feedback" label as it'd be good to get feedback from designers on the desired behaviour of double clicking a list view item. I remember a while back there was some discussion surrounding using a double click to trigger renaming a block, and once we pick a particular behaviour for double click, it might be hard to change it after the fact as folks will have come to expect whichever direction we go with.

One other thing to consider is that when you first click a block in the list view, it will select the block and redirect focus to the editor canvas. Clicking the block again will then shift focus to the list view. However, with double click behaviour added, it's possible that sometimes folks might be attempting to direct focus to the list view and could (potentially) find it unexpected if a list item is expanded or collapsed?

That might not be an issues at all in practice, though, just a thought.

Let's see what some designers think!

@andrewserong andrewserong requested a review from a team August 22, 2024 00:06
@jasmussen
Copy link
Contributor

Thanks for the PR! I could go either way on this one. My only reservation is that once we apply double-click to open/close, we won't be able to apply doubleclick for anything else, such as scroll to the block in question or even open the rename block modal. If we're sure open/close is the best use of this gesture, can work.

@jameskoster
Copy link
Contributor

Personally I would expect a double-click to invoke renaming.

@t-hamano
Copy link
Contributor

This is just one idea, but considering that double-clicking might be used for other functions such as renaming in the future, what do you think about the approach of "expanding if the selected block is single-clicked"?

diff --git a/packages/block-editor/src/components/list-view/block.js b/packages/block-editor/src/components/list-view/block.js
index f4dcf8caef..ea047ba308 100644
--- a/packages/block-editor/src/components/list-view/block.js
+++ b/packages/block-editor/src/components/list-view/block.js
@@ -546,7 +546,13 @@ function ListViewBlock( {
                                        <div className="block-editor-list-view-block__contents-container">
                                                <ListViewBlockContents
                                                        block={ block }
-                                                       onClick={ selectEditorBlock }
+                                                       onClick={ ( event ) => {
+                                                               if ( isSelected ) {
+                                                                       toggleExpanded( event );
+                                                               } else {
+                                                                       selectEditorBlock( event );
+                                                               }
+                                                       } }
                                                        onContextMenu={ onContextMenu }
a694f85e8abae2a4e554f6847280b889.mp4

@jasmussen
Copy link
Contributor

what do you think about the approach of "expanding if the selected block is single-clicked"?

That could potentially interfere with multiselecting. I.e. maybe you'll select a group, hold shift, and then select another block farther down the list.

@nirav7707
Copy link
Contributor Author

That could potentially interfere with multiselecting. I.e. maybe you'll select a group, hold shift, and then select another block farther down the list.

@jasmussen Right, it's prevent deselection of block once we have selected multiple blocks by holding shift. we have to click somewhere inside the block previewer.

multiselect.mov

@nirav7707
Copy link
Contributor Author

By adding bool variable isMultipleBlocksSelected we can prevent the conflict with multiselection issue

	const selectedBlockCount = useSelect((select) => {
		const { getSelectedBlockClientIds } = select(blockEditorStore);
		return getSelectedBlockClientIds().length;
		}, []);
	
	const isMultipleBlocksSelected = selectedBlockCount > 1;

<ListViewBlockContents
	block={ block }
	onClick={ ( event ) => {
			if ( isSelected && ! isMultipleBlocksSelected ) {
					toggleExpanded( event );
			} else {
					selectEditorBlock( event );
			}
	} }
	onContextMenu={ onContextMenu }
Screen.Recording.2024-08-22.at.3.58.57.PM.mov

@richtabor
Copy link
Member

but considering that double-clicking might be used for other functions such as renaming in the future

I'd rather double-click to rename, as that's a much more difficult task to complete than to open/close a block. Does this effort block that potential in the future?

@t-hamano
Copy link
Contributor

By the way, I think there are three actions to rename a directory in Mac OS, Am I right?

  • Select a directory and hit Enter
  • Select a directory and open the context menu
  • (Models with pressure-sensitive trackpads only) Click with force

In Windows OS, there are three actions for renaming:

  • Select a directory and press F2 key
  • Select a directory and single-click on it with the mouse
  • Select a directory and open the context menu

The reason I want to confirm this is because adding a shortcut for renaming is requested in #62152, and if we can implement a shortcut for renaming, we might be able to proceed with this PR.

@richtabor
Copy link
Member

By the way, I think there are three actions to rename a directory in Mac OS, Am I right?

You can also double-click.

@paaljoachim
Copy link
Contributor

paaljoachim commented Aug 22, 2024

I have myself gone a bit forth and back on this feature.
Should double clicking open or rename.....?
Because of the uncertainty around this feature I would likely hold off on this.
As I do believe additional features will be added to List view that might also impact renaming and opening of inner blocks.

It would likely be useful to check into other software that would do something similar and compare. (Besides OSX that is.)

@t-hamano
Copy link
Contributor

Should double clicking open or rename.....?
It would likely be useful to check into other software that would do something similar and compare. (Besides OSX that is.)

It might be good to research and discuss this point first.

I believe the unintended behavior when multiple blocks are selected can be addressed by checking whether a modifier key is pressed, or by checking the value of isMultiBlocksSelected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] List View Menu item in the top toolbar to select blocks from a list of links. Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

double-click to open/close block in List View
8 participants