Site Editor Hub: Simplify #61579
Merged
Site Editor Hub: Simplify #61579
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
youknowriad
added
the
[Type] Technical Prototype
|
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @dereksmart. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
youknowriad
commented
May 10, 2024
| isTransparent={ | ||
| isResizableFrameOversized | ||
| } | ||
| /> | ||
| <SidebarContent routeKey={ routeKey }> | ||
| { areas.sidebar } | ||
| </SidebarContent> |
There was a problem hiding this comment.
I think we can probably simplify more here in terms of styles and classes.
youknowriad
commented
May 10, 2024
| onClick={ () => setCanvasMode( 'view' ) } | ||
| > | ||
| <SiteIcon className="edit-site-layout__view-mode-toggle-icon" /> | ||
| </Button> |
There was a problem hiding this comment.
The other good thing here is that this makes the header identical to the post editor and unifying this button is now possible.
|
Size Change: -284 B (-0.02%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
|
Quick note that I tried to use WP playground to test this but I can't see the icon when I'm in the canvas to get a better sense of it: icon.missing.mov |
|
@annezazu looks like a bug, let me check. |
|
Thank you! Here's a little before/after video to show the differences: before.after.pr.icon.movHere's the prior PR afaik that animated this: #48886 I do think some level of magic and polish is lost with this animation gone. Thinking ahead to more personalization efforts with the admin redesign, I imagine this same site "hub" will continue to play a big role to orient folks and some level of animation will go a long way to convey a smoothness and cohesiveness of the experience. |
|
I agree that the animation is nice but what I'm saying is that the way it's implemented today (keeping the same DOM element) is way too complex for what it adds and unless we find a better way to do it, it's not worth it. An example of this is the distraction free bug here #60875 I think "view transitions" would be perfect here. They already work on Chromium browsers (Chrome, Edge) but not yet in Safari and Firefox. It can be done as a progressive enhancement though. |
|
I pushed a quick prototype for view transitions in Chrome. You can see now in Chrome that the "site icon" is animated. The animation is slightly different than what we have in trunk (I believe we can make it similar to trunk if we really want to) but I'm really excited about this. It's huge improvement over the gymnastic we need to do in our code base to implement animations today. Also for site hub specifically, it can be acceptable to leverage this as a progressive enhancement: works on Chromium browsers (more than 71% of all users) while giving up on the hub animation for the rest for now and keeping the simplicity of our code base and fixing the distraction free mode bug. |
youknowriad
force-pushed
the
update/refactor-site-editor-hub
branch
from
dd733dc to
1af9916
Compare
|
Tested again and it doesn't feel very smooth or polished compared to before: site.hub.animatino.movThis is especially the case when returning to the overall navigation where the site icon zooms in almost too much. When entering the canvas, it feels delayed. I'm not sure if there's a way to make it more seamless while still getting the benefits of this PR as I definitely want to see this bug resolved #60875. As is though, this PR isn't quite there. |
youknowriad
force-pushed
the
update/refactor-site-editor-hub
branch
from
1af9916 to
e6f8f78
Compare
|
I updated this PR to delay only one side of the animation. I think this is way better. Thoughts? |
draganescu
reviewed
May 28, 2024
|
@draganescu what's broken exactly in Firefox? For me it's working, so I wonder if we're talking about a different unrelated bug. |
youknowriad
force-pushed
the
update/refactor-site-editor-hub
branch
from
e6f8f78 to
bfe6568
Compare
|
@youknowriad as the video shows in Firefox when the edit mode is entered and DFM is on the interface header contains no buttons and stays there empty. On this PR in other browsers the current bug in trunk where the header does not appear at all in DFM is fixed. So the fix is incomplete somehow in FF and this PR introduces the problem in FF. |
|
@draganescu I can reproduce that bug in any browser, it depends on where you click to enter "edit mode". I suspect that it's another bug unrelated with the current fix but I'll take a look anyway. |
|
Ok, so the bug is due to multiple header animations conflicting with each other, I solved it by simplifying and unifying the header animation in a single place. |
jeryj
reviewed
May 28, 2024
|
@jeryj I'm guessing that you're thinking about the move from "edit" to "view" mode right? because for the opposite direction, it's probably the same as trunk. |
youknowriad
referenced
this pull request
May 28, 2024
Co-authored-by: WunderBart <bartkalisz@git.wordpress.org> Co-authored-by: ellatrix <ellatrix@git.wordpress.org>
|
Yes, edit to view mode. I forgot that going from view to edit, you enter by selecting the canvas, which places focus on the canvas correctly. |
…s that support view transitions
youknowriad
force-pushed
the
update/refactor-site-editor-hub
branch
from
6c58262 to
e70b9f2
Compare
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
Unlinked contributors: dereksmart. Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
ellatrix
pushed a commit
that referenced
this pull request
Jun 11, 2024
Unlinked contributors: dereksmart. Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
12 tasks
|
I'm here to invite anyone still subscribed to have a look at #62386 which follows up on this by restoring the previous transition. To me, the transition after this is too degraded to overlook. I tried various ways of fixing it and that’s what seems best to me. |
patil-vipul
pushed a commit
to patil-vipul/gutenberg
that referenced
this pull request
Jun 17, 2024
Unlinked contributors: dereksmart. Co-authored-by: youknowriad <youknowriad@git.wordpress.org> Co-authored-by: draganescu <andraganescu@git.wordpress.org> Co-authored-by: jeryj <jeryj@git.wordpress.org> Co-authored-by: richtabor <richtabor@git.wordpress.org> Co-authored-by: annezazu <annezazu@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: Mamaduka <mamaduka@git.wordpress.org> Co-authored-by: stokesman <presstoke@git.wordpress.org> Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
ellatrix
removed
the
Backport to WP 6.7 Beta/RC
|
This was cherry-picked to the wp/6.6 branch. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What and Why?
Currently the site editor renders a "hub" with a site logo, site title and some actions. That hub is persistent and rendered as "fixed" on top of everything.
I think the main reason it's implemented that way is to be able to animate the site logo with go in and out of view mode. That said, I personally believe this is way too complex to accommodate for a small animation.
So this PR is kind of a provocation, how simple can it be if we don't have to implement the animation of the logo. It's not necessarily meant to be merged but at least I want to raise this discussion.
Also, coincidentally, this fixes #60875
Let me know what you think.