Visual revisions: add shareable urls - #79934
Conversation
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @priethor! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
|
Size Change: +1.23 kB (+0.02%) Total Size: 7.77 MB 📦 View Changed
|
Indeed, it was very annoying 😅 . Fixed it in #79987 |
…ons-shareable-urls # Conflicts: # packages/router/CHANGELOG.md
jorgefilipecosta
left a comment
There was a problem hiding this comment.
Left some minor comments but things tested well for me.
| 'wp_template_part', | ||
| ].includes( postType ); | ||
|
|
||
| useEffect( () => { |
There was a problem hiding this comment.
I don't understand why this is an effect. Why don't we do this when the revisions button is clicked, or determine the right link to provide for the button?
There was a problem hiding this comment.
The button already links to revision.php when clicked, indeed, but the effect handles opening an editor URL that already contains ?revision= in it. In that case, we only know after the editor loads whether the post needs the classic revisions screen or not, and once we know, the effect redirects there. I added a comment to make that clearer.
There was a problem hiding this comment.
Why do we only know that when the editor loads? Why can't we redirect in PHP?
| const _isPreviewingTheme = isPreviewingTheme(); | ||
| const iframeProps = useEditorIframeProps(); | ||
| const isEditMode = canvas === 'edit'; | ||
| useRevisionsURLSync( isEditMode, editorPostType, editorPostId ); |
There was a problem hiding this comment.
Just seeing this already makes me wonder: is the URL not the source of truth? If not, why not? Why are we syncing it?
There was a problem hiding this comment.
Right now, the URL sets the initial revision when an entity loads. After that, the existing revisions UI keeps the selection and this hook mirrors those changes back to the URL.
Making the URL the only source would mean changing the shared revision controls so every selection goes through navigation, especially while using the slider. I’m not against that, but it felt like a bigger behavioral change and a lot of navigation for this PR. What do you think?
There was a problem hiding this comment.
Making the URL the only source would mean changing the shared revision controls so every selection goes through navigation, especially while using the slider.
Shouldn't that be the case? The URL is the state. What's the behavior change?
There was a problem hiding this comment.
The behavior change is mostly in the slider: the preview updates immediately, while URL updates have to be throttled because of Safari’s History API limit. Making the URL the source of truth would be cleaner indeed, but it would also need some changes to the shared revision controls. I’d rather handle that in a polishing follow-up, would that work for you?
There was a problem hiding this comment.
Sure. Isn't the slider already throttled?
There was a problem hiding this comment.
Nope, but it might be worth considering handlding the throttle at RangeControl level. 🤔
There was a problem hiding this comment.
I thought I throttled a tiny bit because otherwise it goes through tons of diff calculations of intermediate revisions you slide through.
There was a problem hiding this comment.
If it does, I can't see it 😅 . The slider seems to dispatch on every input event, so dragging accross 20 revisions would parse all 20, not just the one you land in.
I guess moving up the throttle a level (even if it's not at range control level) would improve the revisions performance to avoid these unnecessary diffings when sliding.
There was a problem hiding this comment.
I guess moving up the throttle a level (even if it's not at range control level) would improve the revisions performance to avoid these unnecessary diffings when sliding.
To answer my comment above: I measured it to see how impactful it could be with 30 revisions of ~100 paragraphs each and it takes under 50 ms for a full drag, so I don't think it's worth considering throttling at all.
There was a problem hiding this comment.
Right, I think I ended up speeding up diffing and dropped the throttling maybe. 🤔 100 paragraphs is small though.
|
This PR is a blessed task, so we are considering backporting it to 7.1. Do you have any concerns about backporting this PR as is to 7.1? cc @annezazu |
I'd like to explore a follow-up based on the feedback received to make the URL the canonical source of truth. I don't consider it a blocker to port it and start testing it, but also don't have a strong opinion on whether it should be included at this point, I'll defer to release leads. |
|
It has been agreed to backport this PR to 7.1. If a follow-up PR is submitted, let's backport it to 7.1 as well, if possible. |
|
There was a conflict while trying to cherry-pick the commit to the wp/7.1 branch. Please resolve the conflict manually and create a PR to the wp/7.1 branch. PRs to wp/7.1 are similar to PRs to trunk, but you should base your PR on the wp/7.1 branch instead of trunk. |
Co-authored-by: priethor <priethor@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: aduth <aduth@git.wordpress.org>
|
Manual backport: #81205 |
Co-authored-by: priethor <priethor@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: tyxla <tyxla@git.wordpress.org> Co-authored-by: getdave <get_dave@git.wordpress.org> Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: t-hamano <wildworks@git.wordpress.org> Co-authored-by: aduth <aduth@git.wordpress.org>
What
Part of #79120, closes #76552.
Makes visual revisions linkable in the post editor and site editor.
Why
Reviewing or discussing a specific revision is much easier when the URL points to the exact revision being shown.
How
revision=<id>now opens the editor directly on that revision, and moving through revisions keeps the address bar up to date so the current view can be shared.Testing Instructions
wp-admin/post.php?post=<post-id>&action=edit&revision=<old-revision-id>.revisionarg changes to the selected revision.revisionarg is removed.revisionID and check that the notice appears and the URL is cleaned up.wp-admin/site-editor.php?p=%2Fpage%2F<page-id>&canvas=edit&revision=<old-revision-id>.Screencast
Grabacion.de.pantalla.2026-07-08.a.las.12.12.25.mov
Implemented with Claude and Codex, with direction, manual testing, and review by me.