Skip to content

Conversation

@talldan
Copy link
Contributor

@talldan talldan commented Jul 15, 2025

What?

Adds two options to dataviews actions:

  • hideItemAction - makes an action not appear in an individual item's action dropdown.

  • isTextButton - when an action appears in the footer, this makes it show as a text button.

Screenshot 2025-07-15 at 11 28 18 am

Why?

The goal of these changes is to support experiences where dataviews can be used as a 'picker'. For example, a UI for choosing a parent post or choosing a featured image.

Before this PR, you could kind of implement this behavior, but with two issues:

  • The action for confirming a selection doesn't make sense for individual items (hence hideItemAction)
  • The bulk action in the footer for confirming a selection only appeared as an icon button, not prominent enough for a main CTA.

I've added two new stories 'Single selection' and 'Multi selection' to demonstrate how it might work.

How?

  • Adds the two options
  • Adds to the logic for hiding the row actions when there are no eligible action, taking hideItemAction into account.
  • Makes text buttons use primary styling when the isPrimary option is used on an action (but this change doesn't affect icon buttons).

Testing Instructions

  1. Check out this branch locally
  2. Run npm run storybook:dev to start the story book
  3. Navigate to the DataViews stories, and try the Single Selection and Multi Selection stories.
  4. Note that it's best to use the checkbox for multiselection. I've reported a possible bug around clicking the row - DataViews: Multi-selection works inconsistently in table layout

Screenshots or screencast

Kapture.2025-07-15.at.11.41.42.mp4

@talldan talldan self-assigned this Jul 15, 2025
@talldan talldan added [Type] Enhancement A suggestion for improvement. [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Jul 15, 2025
@talldan talldan requested a review from oandregal July 15, 2025 03:45
@talldan
Copy link
Contributor Author

talldan commented Jul 15, 2025

For hideItemAction I notice there's also this issue proposing a different API for hiding actions to solve a separate issue - DataViews: Remove primary actions from secondary actions drop down.

It's a somewhat similar concern though, so maybe a single unified API for both problems would be better. It might require some more thought/feedback so that we don't end up with multiple confusing APIs for similar problems.

Here's an idea for an API:

// uses 'sensible defaults' when undefined
visibility: undefined, 

// appears only in primary area
visibility: [ 'primary' ], 

// appears only in secondary area
visibility: [ 'secondary' ], 

// appears only in the bulk selection area
visibility: [ 'bulk' ], 

// options can be combined
visibility: [ 'bulk', 'secondary' ] 

It could replace the existing isPrimary and possibly also supportsBulk APIs.

@github-actions
Copy link

github-actions bot commented Jul 15, 2025

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: talldan <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: andrewserong <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: jasmussen <[email protected]>

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

@andrewserong
Copy link
Contributor

This is testing well for me so far!

A couple of tiny observations. One, which isn't caused by this PR, but I just noticed it, is that in the Grid view, when no secondary actions are visible, it looks like we disable the dropdown 3-dots icon, but still render it: (in the list and table views, it looks like it's hidden rather than disabled):

image

In terms of the API, a couple of quick thoughts (I don't have a strong opinion here!)

  • Do we need isTextButton or could we infer it by the absence of an icon defined for the action? Or are you imagining that we might need the icon for display in the secondary actions?
  • I like the idea of visibility rather than a proliferation of props, but I also see isPrimary is used for filters, etc, so there's some symmetry there. There's also a context prop here which says that it's about the context in which the action is visible with examples of list or single. If we go with the idea of visibility (or something like it), I wonder if it should be combined with context?

@talldan
Copy link
Contributor Author

talldan commented Jul 15, 2025

Do we need isTextButton or could we infer it by the absence of an icon defined for the action? Or are you imagining that we might need the icon for display in the secondary actions?

Yep, also that it might also be used if the implementor wants a button with both text and an icon in the bulk actions footer.

I also see isPrimary is used for filters, etc, so there's some symmetry there. There's also a context prop here which says that it's about the context in which the action is visible with examples of list or single. If we go with the idea of visibility (or something like it), I wonder if it should be combined with context?

Just did a bit more digging and found the originating PR for context - #62443. I need to look into it a bit more closely.

Another thing to note is that there's also some overlap between the naming and the variations on a button (primary / secondary / tertiary). In this PR I'm using isPrimary to give the text button a variation, which is probably a misuse of the API.

@talldan
Copy link
Contributor Author

talldan commented Jul 15, 2025

Just did a bit more digging and found the originating PR for context - #62443. I need to look into it a bit more closely.

I think it's this:
list - makes the action only available in dataviews listings
single - makes the action only available in the document panel within the editor (but contextually I guess whenever one of those menus might be present while viewing only a single item).

We could consider also adding an option bulk to this to support the use case for this PR. Not sure if that's stretching the API, but it's also a private api.

@talldan talldan requested a review from mcsf July 15, 2025 07:06
@youknowriad
Copy link
Contributor

I think using "DataViews" for pickers is something we didn't think about much. Is it really something we want to support in the first place? cc @jasmussen @jameskoster

The other question, is more technical, if we do want to support this, I think we should take a step back and thing more globally. Does it make sense to actually use "bulk actions" or "actions" for picking items, or is this a special behavior? Is this actually really the DataViews component or a DataPicker component that could reuse a lot of the layouts... but with a more specific behavior.

@jasmussen
Copy link
Contributor

I think using "DataViews" for pickers is something we didn't think about much. Is it really something we want to support in the first place?

I'd like to invert the question: if we don't use DataViews for pickers, what would we use instead? And wouldn't we end up needing quite a lot of the same features?

For what it's worth, I think it makes sense because DataViews already supports multi select with contextual actions, and if DataViews were to eventually be adopted by a media library, for example, I could see it useful to use actions in this PR for things like "download all" after having selected x images. From that POV, a "picker" is mostly the inverse. I would even expect drag/dropping multiple images into a media library dataview to stay there, and handle the upload, immediately having all my uploaded files pre-selected.

From a superficial glance, using the same componentry for both views and pickers appears to be what both Google Drive and Dropbox do when you use their respective insert from Drive/Dropbox features. There is perhaps an opportunity for a more compact variant of DataViews that works better for the picker context. Coming to mind, uploading a site logo to the site logo block currently opens a massive media library. That could benefit from being a smaller modal flow. That's just speculation and nothing to consider for this PR.

@youknowriad
Copy link
Contributor

I know @ellatrix tried to use the DataViews as a picker in the hosting dashboard and reverted to a custom UI component I think.

I'm not saying we shouldn't use DataViews as a Picker, I'm saying that they were not built for this use-case in mind. They were mostly built for rendering and visualizing lists. If we want to support that, we need to think more globally and not hack things together. For instance, does the Aria model of dataviews fit the "picker" model? Do "bulk actions" make sense for pickers or the selection itself is actually the "picking" and we'd just need a "Select" button that is outside the DataViews component (so not an action)

@talldan
Copy link
Contributor Author

talldan commented Jul 16, 2025

At the moment it's my first time working with dataviews, so I don't know the history, that's the main reason why I'm trying to add a lot of reviewers that can help answer with that side of things.

My feeling is that we could create something alternative, but I think it'd end up being a very close replica to dataviews with only a few small differences. Search, pagination, selection are all things that are already built for dataviews We could also consider more specific views/layout types for this purpose if the existing views don't fit.

Do "bulk actions" make sense for pickers or the selection itself is actually the "picking" and we'd just need a "Select" button that is outside the DataViews component (so not an action)

A bulk action is required for selection to work within dataviews (for the checkboxes to show). That requirement could be lifted, and then I think it's also a valid option.

Comment on lines +172 to +178
label={ action.isTextButton ? undefined : label }
children={
action.isTextButton ? ( label as ReactNode ) : undefined
}
variant={
action.isPrimary && action.isTextButton ? 'primary' : undefined
}
Copy link
Member

Choose a reason for hiding this comment

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

I don't have a lot of DataViews fu, so take these questions with lots of salt.

My first question was "what is isTextButton"? The name suggests it's about what type of button this is, but it's actually about how the button should be displayed.

Are the Button component's in-built props not flexible enough to render a text button vs an icon button? E.g., text , children and label?

Could we rather open up the Button's props and allow passing a variant?

I'm not sure, I know you've probably thought about this anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is just how the Button works afaik. label is just for the aria-label or tooltip text, and mostly used for icon buttons.

For a text button children is used. I suppose I could change it to <Button>{ label }</Button> instead of <Button children={ label } />. 😄

Copy link
Member

Choose a reason for hiding this comment

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

I'll take your word for it!

Or return different Button components? I dunno, I'm just saying random stuff.

	if (hasIcon && hasChildren) {
		// Show both icon and text
		return (
			<Button
				{...buttonProps}
				icon={action.icon}
				label={label}
			>
				{children}
			</Button>
		);
	} else if (hasIcon) {
		// Show icon with tooltip
		return (
			<Button
				{...buttonProps}
				icon={action.icon}
				label={label}
				showTooltip
			/>
		);
	} else {
...

@youknowriad
Copy link
Contributor

My feeling is that we could create something alternative, but I think it'd end up being a very close replica to dataviews with only a few small differences. Search, pagination, selection are all things that are already built for dataviews We could also consider more specific views/layout types for this purpose if the existing views don't fit.

Just to clarify, one of my suggestions was not to create a replica, but a new component in the DataViews package that would have an API tailored for selection. Internally, it would use the exact same sub component used in DataViews.

In other words, before implementing and trying to hack things around, I'd ask myself, what would be the ideal UX and DevX (API) for a generic "picker" based on data and work from there.

@talldan
Copy link
Contributor Author

talldan commented Jul 17, 2025

Just to clarify, one of my suggestions was not to create a replica, but a new component in the DataViews package that would have an API tailored for selection. Internally, it would use the exact same sub component used in DataViews.

In other words, before implementing and trying to hack things around, I'd ask myself, what would be the ideal UX and DevX (API) for a generic "picker" based on data and work from there.

I like this suggestion. I'll explore this in a separate PR. I can see a lot of benefits as we'll be able to control exactly what consumers try to do with the component(s) (like make sure there's only one action, if we use actions), though I still think we might end up needing to change some DataViews internals to support it.

@talldan talldan closed this Jul 17, 2025
@oandregal oandregal deleted the add/dataviews-action-button-options branch July 18, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants