Skip to content
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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jul 29, 2024
+76 −7
Merged

DataForm: add author to quick edit page/post list #63983

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
271dac7
Add validation to quick edit form
oandregal Jul 26, 2024
23d3904
Add author to quick-edit form
oandregal Jul 26, 2024
16cda97
Display select control for users
oandregal Jul 26, 2024
d88059f
Allow for undefined author
oandregal Jul 26, 2024
216234e
Bring back getValue for author
oandregal Jul 26, 2024
f3053ac
If elements are provided, the value needs to be one of them
oandregal Jul 26, 2024
1dff635
Update storybook
oandregal Jul 26, 2024
9bfa200
Remove unnecessary getValue from author field
oandregal Jul 29, 2024
70e6966
Convert to number before validating
oandregal Jul 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add author to quick-edit form
@oandregal
oandregal committed Jul 26, 2024
commit 23d3904261bc14831c7409236f1da2cc1f6492a9
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/post-edit/index.js
View file
Original file line number Diff line number Diff line change
@@ -42,7 +42,7 @@ function PostEditForm( { postType, postId } ) {
const { saveEntityRecord } = useDispatch( coreDataStore );
const { fields } = usePostFields();
const form = {
visibleFields: [ 'title' ],
visibleFields: [ 'title', 'author' ],
};
const [ edits, setEdits ] = useState( {} );
const itemWithEdits = useMemo( () => {
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/post-fields/index.js
View file
Original file line number Diff line number Diff line change
@@ -235,7 +235,7 @@ function usePostFields( viewType ) {
{
label: __( 'Author' ),
id: 'author',
getValue: ( { item } ) => item._embedded?.author[ 0 ]?.name,
Copy link
Member Author

@oandregal oandregal Jul 26, 2024 •
edited
Loading

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).

Copy link
Contributor

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?

Copy link
Member Author

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

Copy link
Member Author

@oandregal oandregal Jul 26, 2024 •
edited
Loading

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)

Copy link
Member Author

@oandregal oandregal Jul 29, 2024 •
edited
Loading

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.

type: 'integer',
elements:
authors?.map( ( { id, name } ) => ( {
value: id,