-
Notifications
You must be signed in to change notification settings - Fork 4.6k
DataViews: allow hiding config #71173
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
Conversation
|
A small detail, but is ‘Non interactive’ the right way to frame this? Elements within the dataview can still be interactive, right? For instance a consumer could create a ’non interactive’ view where the primary field still links somewhere, and records can still support quick actions? Maybe something like ‘Locked view options’ better captures the story. |
The current name "Non-interactive" is something I introduced today in the storybook, before it was "Fields No Sortable No Hidable", which was even less expressive of the intent. Changing it is independent of this PR, so I've prepared #71178 Note I went with "Locked" in #71178 because it's not exactly "locking a view", but it highlights a few things together: making fields no sortable/hidable/filterable, not providing primary/media/description fields, disabling search, disabling all layouts but one, disabling items per page, etc. — all those things were already possible. The original intent of the story was to capture "the minimal DataViews UI you can get". Happy to rename to any other word that best captures this! |
| const { view, config, onChangeView } = useContext( DataViewsContext ); | ||
| if ( | ||
| ! config || | ||
| ! config.perPageSizes || |
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.
Is this is a change in behaviour? Previously perPageSizes had some default values but these are no longer set? [ 10, 20, 50, 100 ]
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.
Thanks, I missed that. They've been added where they were (see below).
mikejolley
left a comment
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.
Took a peek in storybook and see no search controls, view menu etc above the DataView. I assume thats intended? Are we going to enforce config to show any of those areas and how much control over items in the view menu will be provided?
Search is hidden by its own DataViews prop ( The granular control over what to display within the view config hasn't changed (the |
a81e8d7 to
bfb0940
Compare
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in bfb0940. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/16940488164
|
|
@jameskoster @mikejolley how does this look? do we go ahead? |
|
Seems good to me :) |
bfb0940 to
7e6a78c
Compare
Why is this use-case not addressed by the "composable dataviews" instead? |
That approach requires the consumer to compose the DataViews from scratch to only disable the cog button. What would be the advantage? Note that we were already configuring the "view config" (perPageSizes) via a DataViews prop. Given potential needs to control more of the view config behavior, grouping all of that into a |
In this particular case, I think it's fine because the user only needs to render one component (the layout) within the provider, so DevX wise, it seems ok no? |
|
My concern here is that how much of this type of flags/.... do we want, someone might want later to hide just "filters" or other just the "layout switcher".... I think if we start doing stuff like that, we'll never stop and the composition approach is more flexible and allow all of these. |
All of this was already possible:
We also already allowed to hide the Using the "free form" approach for this case is fragile and comes with its own issues. This is what you have to do to replicate the same: <DataViews>
<DataViews.Layout />
<HStack expanded={false} justify="end" className="dataviews-footer">
<DataViews.Pagination />
</HStack>
</DataViews>Note the use of internal DataViews classes (become public API) to have the same spacing, etc. And this isn't a complete example: it misses BulkActions, at which point you need to replicate the whole DataViews Footer. And if any of that changes in a new DataViews release, consumers need to catch up. I agree "free form" is great, and we shouldn't try to replicate it via DataViews pros. In this particular case, I feel it is fine to have a |
|
Could it make sense to create |
I've implemented this at #71276 |
What?
This PR explores making the view config hidable:
Why?
There are some cases in which DataViews would benefit from not providing any controls other than the data itself. A big part of this is already possible through the existing props, the major offender is the view config cog (see "Minimal UI" story).
I've noticed that we've already introduced some props that control the view config (items per page). We can group them under a "view config" prop and so if/when we want to add more configurability, we'd have a place for growing without expanding top-level DataViews props ad infinitum.
How?
configprop to DataViews. Name TBD. It can befalseor an object.perPageSizesto that newconfigprop.Testing Instructions
Visit the storybook (
npm install && npm run storybook:dev) and try the default (per page sizes control) and the minimal UI stories.