WIP: Posts list #62705
Draft
WIP: Posts list #62705
+268
−139
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ntsekouras
added
[Feature] Data Views
ntsekouras
requested review from
mcsf,
youknowriad,
oandregal,
jameskoster,
jasmussen,
ellatrix and
jorgefilipecosta
|
Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.
Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task. |
|
Size Change: +153 B (+0.01%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 7493f95. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9599370698
|
youknowriad
reviewed
Jun 20, 2024
| }; | ||
| function PostsLayout() { | ||
| // This ensures the edited entity id and type are initialized properly. | ||
| useInitEditedEntityFromURL(); |
There was a problem hiding this comment.
How hard would it be to move away from this hook and instead of having postId postType and context in the site editor store, just pass it down as props to the Editor component. (For the site editor, not the posts page, we could keep syncing these to the store for backwards compatibility)
youknowriad
reviewed
Jun 20, 2024
| @@ -1,3 +1,6 @@ | |||
| // TODO: we need to examine if these styles could be the default styles | |||
| // for every post type and/or how to have different styles for different post types. | |||
| // Example is `posts` and `pages`. | |||
There was a problem hiding this comment.
The classnames seems wrong now right?
youknowriad
reviewed
Jun 20, 2024
| preview: ( isListLayout || canvas === 'edit' ) && ( | ||
| <Editor isLoading={ isSiteEditorLoading } /> | ||
| ), | ||
| mobile: | ||
| canvas === 'edit' ? ( | ||
| <Editor isLoading={ isSiteEditorLoading } /> | ||
| ) : ( | ||
| <PagePages /> | ||
| <PostsList postType={ postType } /> |
There was a problem hiding this comment.
This is the kind of change I was hoping to see :)
youknowriad
reviewed
Jun 20, 2024
| ); | ||
|
|
||
| // Posts list. | ||
| if ( [ 'page', 'post' ].includes( postType ) ) { |
There was a problem hiding this comment.
Just a lot of temp code to get first feedback. These should be generic eventually and we could even restrict this for posts as a first step..
youknowriad
reviewed
Jun 20, 2024
| const isSiteEditorLoading = useIsSiteEditorLoading(); | ||
| const { params = {} } = useLocation(); | ||
| const { postType, layout, canvas } = params; | ||
| const labels = useSelect( |
There was a problem hiding this comment.
I would prefer if we avoid useSelect and complex logic in the "router". Ideally this is actually just a global object (when we'll be able to define "paths" as part of the config). So I'd actually move these calls to dedicated components that I render in areas.
youknowriad
reviewed
Jun 20, 2024
| const { useLocation } = unlock( routerPrivateApis ); | ||
|
|
||
| export default function useLayoutAreas() { | ||
| const isSiteEditorLoading = useIsSiteEditorLoading(); |
There was a problem hiding this comment.
Already existing in the other router, but maybe we can just move this to the "Editor" component now?
There was a problem hiding this comment.
Yeah, I have this in mind for things that end up being reused, to be moved elsewhere.. Just temp code for first feedback.
Good question, I'm thinking we use the same "component", it doesn't matter if we call it post or site editor. That component will probably be the site editor one for a start because we're building this stuff in the site editor package today. The Global Styles question is a good one, I don't think we need to get everything right in the first PR. I think it's fine to keep it as is for now.
Good question as well, maybe "posts" is a top level menu item and you gave to go into it first (like pages in the site editor) but it raises the question of what is the "front page" of this experience. Also a design question mainly that can be addressed in a follow-up.
Definitely 💯 |
That's the key bit. It can live here fo now:
But in order to solve the structural questions it does need to move to the top level. It's a good start, but I am seeing some errors, for awareness: |
| isResolving: isLoadingPages, | ||
| totalItems, | ||
| totalPages, | ||
| } = useEntityRecords( 'postType', postType, queryArgs ); | ||
|
|
||
| usePostIdLinkInSelection( selection, setSelection, isLoadingPages, pages ); | ||
| usePostIdLinkInSelection( |
There was a problem hiding this comment.
This can now be removed.
| addNewLabel: getPostType( 'page' )?.labels?.add_new_item, | ||
| canCreatePage: canUser( 'create', 'pages' ), | ||
| labels: getPostType( postType )?.labels, | ||
| // TODO: check what is the proper way to make this work for any post type.. |
There was a problem hiding this comment.
To get the resource (posts, pages etc) we can do this:
const postTypeObject = getPostType( postType );
const resource = postTypeObject?.rest_base || '';
| value: 'future', | ||
| }, | ||
| { | ||
| title: __( 'Scheduled' ), |
There was a problem hiding this comment.
We are repeating the status view definition all the time I guess we can do something like:
const createStatusView = (title, slug, icon, status) => ({
title,
slug,
icon,
view: {
...DEFAULT_POST_BASE,
filters: [
{
field: 'status',
operator: OPERATOR_IS_ANY,
value: status,
},
],
},
});
// Then use it like:
createStatusView(__('Published'), 'published', published, 'publish'),
createStatusView(__('Drafts'), 'drafts', drafts, 'draft'),
// ... and so on
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Part of: #62371
This PR explores the addition of Data Views for the new experimental
postslist underGutenberg->postsmenu.This is just a starting point, but wanted to have it on GH to start some discussions.
What this PR does for now is the start of making an abstraction that could be used for any post type, but we need to do it at least for
postsandpagesfor now. There are many things to be done yet, like changing labels properly per post type in sidebar, lists, add new component, etc..Maybe the notable things for now to discuss are:
postor thesiteeditor? I've started by having as a goal to load the site editor. This has nuances though. Do we want to have for example global styles in a the editor of the post? Additionally the editor adds some extra settings during initialization (initializeEditor) that are needed here too. How can we abstract this to be reused what's needed.categories, tags, etc.. in the sidebar where we would load the lists for the taxonomies. How does this affect the currentviewswe have(drafts, published, etc..)? Should we consider changing placement of these views into the frame, which might also simplify some shared information that were wanted for views. An example is the number of items per view, which is not possible right now.Notes
listview doesn't work well now of course due to the incorrect initialization of the site editor. You can try this a bit though with the other views (grid, table).usePostActionsfor fields. Now we are reusing the exact same code for thepostslist and the existingpageslist in site editor. So testing thepageslist that works as before is also part of this PR to avoid regressions.