-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Consolidate when showing revisions link or action #60194
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. |
Size Change: +29 B (0%) Total Size: 1.72 MB
ℹ️ View Unchanged
|
I agree it doesn't make sense to provide access to the revisions UI when there are no revisions that can be restored. There's a small quirk however for theme-supplied templates and template parts. I suppose it's because a new record is created in the db instead of the theme file being 'edited'. In terms of UX it produces a counter-intuitive experience: if you edit a template supplied by the theme and save, you can immediately 'clear customizations', which suggests there is at least one revision. But there is no way to view revisions (because technically none exist). It's a minor detail, so probably fine to merge. But it'd be good to hear any ideas how we might smooth out this wrinkle. |
@jameskoster who else should we ping to get some more opinions? Maybe @richtabor ? |
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 think it's fine to merge. The problem I described is basically an edge case.
Co-authored-by: ntsekouras <[email protected]> Co-authored-by: jameskoster <[email protected]> Co-authored-by: kathrynwp <[email protected]> Co-authored-by: jasmussen <[email protected]> Co-authored-by: annezazu <[email protected]>
What?
Resolves: #56603
This should be the last part of the issue and this PR consolidates when to show the
view revisions
link/action. It uses the same logic we were using for lists, which is show the link/action if there are more than one revision. This effectively means that we don't show the first revision from empty content, which by the way is a revision that cannot be 'restored'.Testing Instructions