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

Jump to bottom

Add status to quick edit #64398

Merged
merged 9 commits into from
Aug 13, 2024
Merged

Add status to quick edit #64398

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9c296ee
Display status in QuickEdit
oandregal Aug 5, 2024
d62a7d4
Remove custom getValue from status field
oandregal Aug 5, 2024
ea00c2a
Do not show trash status
oandregal Aug 7, 2024
00e2f90
Display status as radio
oandregal Aug 9, 2024
f0af051
Add description to all status but draft
oandregal Aug 9, 2024
c659533
Edit order of fields to be similar to DocumentInspector
oandregal Aug 9, 2024
a13e016
Update date when new status is publish
oandregal Aug 12, 2024
55ff29f
handle status
oandregal Aug 12, 2024
f234031
Update handle status logic
oandregal Aug 13, 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
29 changes: 27 additions & 2 deletions packages/edit-site/src/components/post-edit/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,38 @@ function PostEditForm( { postType, postId } ) {
);
const [ multiEdits, setMultiEdits ] = useState( {} );
const { editEntityRecord } = useDispatch( coreDataStore );
const { fields } = usePostFields();
const { fields: _fields } = usePostFields();
const fields = useMemo(
() =>
_fields?.map( ( field ) => {
if ( field.id === 'status' ) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this personally but I see why you did that. I wonder if this is something that is common: A way to make an "element" readonly basically. If we should absorb it into the API or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm ok shipping this but can we add add an inline comment to explain why we do this and that we should ideally reuse the fields untouched here (so it doesn't become a common practice).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it seems something that should be part of the field and not check specific ids here. Definitely comment about it if we land as is..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of "readonly" elements, but I'm not sure we should make any changes to the API just yet. What I'm seeing here is that parts of the field need to be different depending on whether it's used in the view or in the edit context. For example, the render function for date is the same: in the edit context I'd have it without the "prefix" (Modified/Scheduled/etc.).

return {
...field,
elements: field.elements.filter(
( element ) => element.value !== 'trash'
),
};
}
return field;
} ),
[ _fields ]
);
const form = {
type: 'panel',
fields: [ 'title', 'author', 'date', 'comment_status' ],
fields: [ 'title', 'status', 'date', 'author', 'comment_status' ],
};
const onChange = ( edits ) => {
for ( const id of ids ) {
if (
oandregal marked this conversation as resolved.
Show resolved Hide resolved
edits.status !== 'future' &&
record.status === 'future' &&
new Date( record.date ) > new Date()
) {
edits.date = null;
}
if ( edits.status === 'private' && record.password ) {
edits.password = '';
}
editEntityRecord( 'postType', postType, id, edits );
if ( ids.length > 1 ) {
setMultiEdits( ( prev ) => ( {
Expand Down
40 changes: 32 additions & 8 deletions packages/edit-site/src/components/post-fields/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,36 @@ import Media from '../media';
// See https://github.com/WordPress/gutenberg/issues/55886
// We do not support custom statutes at the moment.
const STATUSES = [
{ value: 'draft', label: __( 'Draft' ), icon: drafts },
{ value: 'future', label: __( 'Scheduled' ), icon: scheduled },
{ value: 'pending', label: __( 'Pending Review' ), icon: pending },
{ value: 'private', label: __( 'Private' ), icon: notAllowed },
{ value: 'publish', label: __( 'Published' ), icon: published },
{
value: 'draft',
label: __( 'Draft' ),
icon: drafts,
description: __( 'Not ready to publish.' ),
},
{
value: 'future',
label: __( 'Scheduled' ),
icon: scheduled,
description: __( 'Publish automatically on a chosen date.' ),
},
{
value: 'pending',
label: __( 'Pending Review' ),
icon: pending,
description: __( 'Waiting for review before publishing.' ),
},
{
value: 'private',
label: __( 'Private' ),
icon: notAllowed,
description: __( 'Only visible to site admins and editors.' ),
},
{
value: 'publish',
label: __( 'Published' ),
icon: published,
description: __( 'Visible to everyone.' ),
},
{ value: 'trash', label: __( 'Trash' ), icon: trash },
];

Expand Down Expand Up @@ -258,11 +283,10 @@ function usePostFields( viewType ) {
{
label: __( 'Status' ),
id: 'status',
getValue: ( { item } ) =>
STATUSES.find( ( { value } ) => value === item.status )
?.label ?? item.status,
type: 'text',
elements: STATUSES,
render: PostStatusField,
Edit: 'radio',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we edit the status in the editor, we do some extra modifications See

gutenberg/packages/editor/src/components/post-status/index.js

Lines 150 to 164 in b12e897

const handleStatus = ( value ) => {
let newDate = date;
let newPassword = password;
if ( status === 'future' && new Date( date ) > new Date() ) {
newDate = null;
}
if ( value === 'private' && password ) {
newPassword = '';
}
updatePost( {
status: value,
date: newDate,
password: newPassword,
} );
};

I wonder how we should deal with that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that's not all of it. We also show the publish date when a user selects future. Probably it makes sense to export privately the control we use in the editor package and reuse it here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And that's not all of it. We also show the publish date when a user selects future.

This for me is a weird interactions and I know designers feel like it's a good thing but I have some doubts about it personally :)

For the handleStatus function, we can either absorb it as an API or move it to the onChange handler of the DataForm instance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding or not more components to the status field can be a follow-up. Were we to do that, we could create a custom Edit function for status.

enableSorting: false,
filterBy: {
operators: [ OPERATOR_IS_ANY ],
Expand Down
Loading