Site Editor > Pages: move view config to the server#76573
Conversation
|
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. |
| // TODO: this data will come from a registry of view configs per entity. | ||
| $default_view = array( | ||
| 'type' => 'table', | ||
| 'filters' => array(), | ||
| 'perPage' => 20, | ||
| 'sort' => array( | ||
| 'field' => 'title', | ||
| 'direction' => 'asc', | ||
| ), | ||
| 'titleField' => 'title', | ||
| 'fields' => array( 'author', 'status' ), | ||
| ); | ||
| $default_layouts = array( | ||
| 'table' => array(), | ||
| 'grid' => array(), | ||
| 'list' => array(), | ||
| ); | ||
| $all_items_title = __( 'All items', 'gutenberg' ); | ||
| if ( 'postType' === $kind ) { | ||
| $post_type_object = get_post_type_object( $name ); | ||
| if ( $post_type_object && ! empty( $post_type_object->labels->all_items ) ) { | ||
| $all_items_title = $post_type_object->labels->all_items; | ||
| } | ||
| } | ||
| $view_list = array( | ||
| array( | ||
| 'title' => $all_items_title, | ||
| 'slug' => 'all', | ||
| ), | ||
| ); | ||
| if ( 'postType' === $kind && 'page' === $name ) { | ||
| $default_view = array( | ||
| 'type' => 'list', | ||
| 'filters' => array(), | ||
| 'perPage' => 20, | ||
| 'sort' => array( | ||
| 'field' => 'title', | ||
| 'direction' => 'asc', | ||
| ), | ||
| 'showLevels' => true, | ||
| 'titleField' => 'title', | ||
| 'mediaField' => 'featured_media', | ||
| 'fields' => array( 'author', 'status' ), | ||
| ); | ||
| $default_layouts = array( | ||
| 'table' => array( | ||
| 'layout' => array( | ||
| 'styles' => array( | ||
| 'author' => array( | ||
| 'align' => 'start', | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| 'grid' => array(), | ||
| 'list' => array(), | ||
| ); | ||
| $view_list = array( | ||
| array( | ||
| 'title' => $all_items_title, | ||
| 'slug' => 'all', | ||
| ), | ||
| array( | ||
| 'title' => __( 'Published', 'gutenberg' ), | ||
| 'slug' => 'published', | ||
| 'view' => array( | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'publish', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| array( | ||
| 'title' => __( 'Scheduled', 'gutenberg' ), | ||
| 'slug' => 'future', | ||
| 'view' => array( | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'future', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| array( | ||
| 'title' => __( 'Drafts', 'gutenberg' ), | ||
| 'slug' => 'drafts', | ||
| 'view' => array( | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'draft', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| array( | ||
| 'title' => __( 'Pending', 'gutenberg' ), | ||
| 'slug' => 'pending', | ||
| 'view' => array( | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'pending', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| array( | ||
| 'title' => __( 'Private', 'gutenberg' ), | ||
| 'slug' => 'private', | ||
| 'view' => array( | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'private', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| array( | ||
| 'title' => __( 'Trash', 'gutenberg' ), | ||
| 'slug' => 'trash', | ||
| 'view' => array( | ||
| 'type' => 'table', | ||
| 'layout' => isset( $default_layouts['table']['layout'] ) ? $default_layouts['table']['layout'] : array(), | ||
| 'filters' => array( | ||
| array( | ||
| 'field' => 'status', | ||
| 'operator' => 'isAny', | ||
| 'value' => 'trash', | ||
| 'isLocked' => true, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| } |
There was a problem hiding this comment.
All this data will come from elsewhere. I want to ship things iteratively to maintain a steady velocity and don't overwhelm reviewers. This follow-up will be tracked at #76544
| key={ view.slug } | ||
| slug={ view.slug } | ||
| title={ view.title } | ||
| icon={ SLUG_TO_ICON[ view.slug ] } |
There was a problem hiding this comment.
The icon is not part of the view-config endpoint because I don't know that we'll need it: 1) there's a experiment (Gutenberg > Experiments > Site Editor extensibility) to update how the site editor looks that doesn't use icons, 2) if we wanted them for any entity, we'll need to figure it out how to use/register them from the server.
I'll get this listed at #76544 so it's tracked.
|
Size Change: +131 B (0%) Total Size: 8.76 MB
ℹ️ View Unchanged
|
71f3119 to
558598d
Compare
|
Flaky tests detected in b50a829. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23258881391
|
|
Prepared #76622 on top of this one to migrate templates. |
| const { set } = useDispatch( preferencesStore ); | ||
|
|
||
| const baseView: View = persistedView ?? defaultView; | ||
| const baseView: View = persistedView ?? defaultView ?? {}; |
There was a problem hiding this comment.
Haven't tested, but I'm curious if an empty object here would cause failures.
There was a problem hiding this comment.
Yes because the user preferences (coming from the me/preferences endpoint) or the default view (coming from the new endpoint) can be in flight when this code runs (undefined) and baseView is used in a bunch of places after.
| 'kind' => array( | ||
| 'description' => __( 'Entity kind.', 'gutenberg' ), | ||
| 'type' => 'string', | ||
| 'readonly' => true, |
There was a problem hiding this comment.
Should we add the context=>array('view') to all of these?
There was a problem hiding this comment.
When we support all targets (pages, templates, patterns, inspector), we'll want to filter the response — not every property is useful/should be returned for every target. However, I don't think context is granular enough for that, and I'd think _fields=... would be a better approach.
lib/compat/wordpress-7.1/class-gutenberg-rest-view-config-controller-7-1.php
Outdated
Show resolved
Hide resolved
| 'field' => 'title', | ||
| 'direction' => 'asc', | ||
| ), | ||
| 'titleField' => 'title', |
There was a problem hiding this comment.
Should we add mediaField->featured_image here?
There was a problem hiding this comment.
I saw you've added below, but that makes me wonder if the default here should not expect the below fields then (author,status).
There was a problem hiding this comment.
Yeah, exactly: this is the default for all post types. Until we absorb also the field declarations and remove this client-side logic, it's not entirely clear to me how/where to leverage the existing post type supports. So I thought it's a bridge we can cross later.
| 'slug' => 'trash', | ||
| 'view' => array( | ||
| 'type' => 'table', | ||
| 'layout' => isset( $default_layouts['table']['layout'] ) ? $default_layouts['table']['layout'] : array(), |
There was a problem hiding this comment.
Before we had the override for author align in view. Here it seems we only do it for trash
There was a problem hiding this comment.
The default view is list, which doesn't provide any layout prop. The layout for table is already set up via $default_layouts. Do you find anything missing? Or the author column field being misaligned?
There was a problem hiding this comment.
the author column field being misaligned?
Yes.
There was a problem hiding this comment.
I was able to repro by:
- Having a view (with user preference: table layout) in trunk
- Switch to this branch
The issue here is that activeViewOverrides for useView is only taking into account the viewList but it should also take into account defaultLayouts. However, it's not trivial to do that programatically accounting for any view type because the view type is not stablished until later (useView) — so we have a recursive dependency. Before, we just added the table layout always, even if type was other than table.
The proper fix would be to actually add support for defaultLayouts in useView but I've tried that before and it didn't get enough support. I'll get back to this.
There was a problem hiding this comment.
In other words: the activeViewOverrides input param of useView needs the view type to compute layout-specific overrides, but it's useView which outputs the view type.
There was a problem hiding this comment.
Considered a few approaches:
- What if
useViewabsorbeduseViewConfigentirely? It works for consumers (single hook call), but I worry thatuseViewbecomes a catch-all hook and composability/performance suffers (some places may only need whatuseViewConfigprovides — without the need to request for user preferences, etc.). - What if we got the type before
useViewusage (via preferences store) and use it to compute the active overrides? It's bad devexp for consumers. - What if we provided all defaultLayouts config to active overrides, ignoring the active view type? The risk is that some layouts have the same prop (e.g., density), and so the last layout destructured in activeOverrides we'll be overriding previous ones. For example, the active layout is table; both table and list have different density, but list is destructured later (density will end up having the list layout value instead of the table layout one):
// pseudocode
const activeOverrides = {
...defaultLayouts.table,
...defaultLayouts.list,
// etc
...viewListOverrides
}All things considered, I think useView handling defaultLayouts is the cleanest. Pushed at b50a829
| $all_items_title = $post_type_object->labels->all_items; | ||
| } | ||
| } | ||
| $view_list = array( |
There was a problem hiding this comment.
I'm not very fond of this name view_list because it feels ambiguous. Not sure for a better name though.. Could just views work?
There was a problem hiding this comment.
views sounds too close to view to make a difference — it's an overloaded term already. Perhaps there's a name in how we call the sidebar/tab list of views? 🤔 I want to avoid to be too much UI centric (tab_list, sidebar_views, etc.) because that ages badly. I don't have a better suggestion than view_list, but happy to iterate (in follow-ups) if there's any.
ntsekouras
left a comment
There was a problem hiding this comment.
Thanks! This looks good overall as a first step!
d8724eb to
05d6187
Compare
ntsekouras
left a comment
There was a problem hiding this comment.
Let's get this in and iterate. Thanks!
|
@ntsekouras @oandregal After this PR, e2e tests in the |
|
That's weird and is probably related to core changes? 🤔 @chriszarate do you know if that is the case or this PR? I'll have a better look tomorrow. |
Yes, that's a much more likely explanation. RTC was turned off by default, we need to enable it explicitly for the RTC e2e tests. |
Part of #76544
Backported at WordPress/wordpress-develop#11272
What?
Move view configuration (
defaultView,defaultLayouts, andviewList) for the Site Editor > Pages screen to a server-side REST API endpoint, replacing client-side definitions.There should be no visual changes.
Why?
See #76544
How?
Server side:
Gutenberg_REST_View_Config_Controller_7_1REST controller underlib/compat/wordpress-7.1/that serves view configuration per entity type via GET/wp/v2/view-config?kind=<kind>&name=<name>.default_view,default_layouts, andview_list.@wordpress/core-data:viewConfigsreducer,receiveViewConfigaction,getEntityViewConfigprivate selector, and resolver that fetches from the new endpoint.@wordpress/views:useViewConfighook that returns the view config (prefered to using the core-data private selector).@wordpress/edit-site:post-list/index.js: UseuseViewConfighook instead of importing fromview-utils.js.sidebar-dataviews/index.js: UseuseViewConfighook instead of importing fromview-utils.js.site-editor-routes/pages.js: Read view config directly from the store instead of importing fromview-utils.js.post-list/view-utils.jsentirely — all its exports are now served by the REST API.Testing Instructions
Use of AI Tools
Claude Code (claude-opus-4-6) was used for code generation and refactoring assistance throughout this PR.