-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
DataForm: add author to quick edit page/post list #63983
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. |
@@ -235,7 +235,7 @@ function usePostFields( viewType ) { | |||
{ | |||
label: __( 'Author' ), | |||
id: 'author', | |||
getValue: ( { item } ) => item._embedded?.author[ 0 ]?.name, |
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.
As far as I'm aware, we didn't really use this function. The implementation is also wrong, because the getValue
should return an integer (what item.author
is) instead of a string (the display name is a render
concern).
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.
Don't we have a filter that uses this?
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.
It's not necessary for filters, using the integer would be enough. However! Your comment makes me realize this is actually necessary for sorting: we should sort authors by name not by the id assigned by WordPress.
I'm going to bring back this, but sorting is not working on trunk
. We need to look at that separately. There's a few issues (author & dates not sorting properly, the sort config in the table headers is not cleared properly):
Gravacao.do.ecra.2024-07-26.as.13.23.26.mov
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.
This actually raises an interesting question: if we have types but sorting depends on getValue, we can end up with the following (like in this case for author
):
- type is
integer
- but
getValue
returns a string
How do we make the sort utility work based on types in this scenario?
It seems we shouldn't use getValue
for sorting and actually provide a custom sort utility if the field needs it. In that case, the filterSort utility would use the custom sort if it exists, otherwise would default to the sort defined by the field type. Thoughts?
(nothing to do in this PR, as I brought back the getValue
function 216234e but wanted to share for consideration when looking at fixing sorting and formalizing types)
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.
I've looked a bit more into this and it turns out getValue
is not used for sorting either: we delegate on the REST API for filter/sort/rest in this page.
The issues I've seen with sorting in trunk
are:
- author is sorted by creation id, not by name. In the existing wp-admin page, users cannot sort by author.
- date: I'm not sure why it doesn't work but it's unrelated. In the existing wp-admin page, this works fine.
I removed the getValue
to proceed with this PR.
Size Change: +423 B (+0.02%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
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.
This is cool 👍
- Definitely something worth adding to the storybook.
- We may want to start to think separately about offering a new "layout" for
DataForm
to render the fields as a panel (like the document panel in the editor)
@@ -235,7 +235,7 @@ function usePostFields( viewType ) { | |||
{ | |||
label: __( 'Author' ), | |||
id: 'author', | |||
getValue: ( { item } ) => item._embedded?.author[ 0 ]?.name, |
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.
Don't we have a filter that uses this?
Next steps here:
|
Flaky tests detected in 1dff635. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10111220218
|
I know what happened with sorting for dates and have a fix at #64049 |
#64064 addresses sorting for author. |
Part of #59745
Follow-up to #63895
What?
Makes the existing
author
field visible in the quick edit form.Gravacao.do.ecra.2024-07-26.as.12.11.44.mov
Why?
To continue expanding the quick edit form.
How?
Testing Instructions