DataViews: Implement isItemClickable and onClickItem props

[gigitux]

What?

This PR fixes #66338.
This PR fixes #66588.

This PR adds two props to the Data Views library:

onClick

This callback is triggered when the user clicks on a media field or a primary field.

isClickable

This function determines if a media field or a primary field are clickable. It takes an item as an argument and returns a boolean value indicating whether the item can be clicked.

Why?

These two callbacks allow developers to implement click handlers for the media field and primary field at the consumer level, rather than within a specific field.

How?

Testing Instructions

Ensure that Quick Edit in DataViews and Quick Edit in DataViews are enabled.

1. Visit the Site Editor
2. Click on pages.
3. Toggle to the table view.
4. Ensure that when you click on the title you are redirected to the page editor
5. Go back.
6. Toggle to the grid view.
7. Ensure that when you click on the image, you are redirected to the page editor.

Testing Instructions for Keyboard

Screenshots or screencast

[youknowriad]

Tested this and it works well for me.

[youknowriad]

Should make this undefined if isClickable is false. Same for onClick prop

[gigitux]

There is an if inside both functions to ensure that the callback is invoked only when the item is clickable.

gutenberg/packages/dataviews/src/dataviews-layouts/hooks/use-clickable-item-props.ts

Lines 25 to 32 in 507d60b

	onKeyDown: ( event: React.KeyboardEvent ) => {
	if (
	( event.key === 'Enter' || event.key === '' ) &&
	isClickable
	) {
	onClickItem( item );
	}
	},

gutenberg/packages/dataviews/src/dataviews-layouts/hooks/use-clickable-item-props.ts

Lines 19 to 23 in 507d60b

	onClick: () => {
	if ( isClickable ) {
	onClickItem( item );
	}
	},

I'm fine with replacing both with something like this if you prefer:

onKeyDown: ! isClickable
? undefined
: ( event: React.KeyboardEvent ) => {
		if ( event.key === 'Enter' || event.key === '' ) {
			onClickItem( item );
		}
  },

onClick: ! isClickable
? undefined
: () => {
		onClickItem( item );
},

[youknowriad]

yeah I just thought avoiding the handlers on things that are not "button" would be best.

[gigitux]

Fixed with c9e418b!

[youknowriad]

I want to acknowledge this is one possible approach, I know you folks are not entirely convinced. I'm not entirely ready to make this layout specific personally.

Would you be ok with moving forward here and judging this over time?

[gigitux]

I want to acknowledge this is one possible approach, I know you folks are not entirely convinced. I'm not entirely ready to make this layout specific personally.

Would you be ok with moving forward here and judging this over time?

The proposed approach works for me. Merging this PR will fix this regression: #66588. We can continue to iterate as needed 💪

[oandregal]

We want these styles to be applied to the title only if the layout supports the item being "clickable". Either we assume these classes become public API (so they should be stable) or we need to figure out better ways to do this.

[oandregal]

Thanks for your work on this, Luigi!

I find a bit problematic that consumers need to target dataviews internal classes (so they become public API). This is an area I'd like us to iterate on.

Other than that, I think we should try this approach to gauge whether it's good enough.

[mcsf]

Why is this needed, and why only for --clickable?

[mcsf]

Is &--clickable the best way to achieve this? We don't seem to do this in Gutenberg (we have only one occurrence of &--is-hex). What does this do that couldn't be achieved by targeting &[role="button"]?

[oandregal]

I agree the way this handles the classes needs to be iterated, see related conversation #66365 (comment)

In discussing with Riad IRL the idea was to land this PR and iterate.

[oandregal]

I want to build on top of this PR, so I've pushed changes to address @mcsf feedback. This PR is not perfect and we need to iterate, but I'm going to merge to unblock related work.

[mcsf]

I want to build on top of this PR, so I've pushed changes to address @mcsf feedback. This PR is not perfect and we need to iterate, but I'm going to merge to unblock related work.

Thanks for addressing most of it, @oandregal! I'm quite fine with merging it as is. Let's address the rest afterwards.