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.

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

Site Editor: Empty header while in distraction-free mode #60875

Open
dereksmart opened this issue Apr 18, 2024 · 10 comments · May be fixed by #61579
Open

Site Editor: Empty header while in distraction-free mode #60875

dereksmart opened this issue Apr 18, 2024 · 10 comments · May be fixed by #61579
Assignees
Labels
[Feature] Distraction Free A preference in the Post and Site Editor that limits distractions to focus the editing experience [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@dereksmart
Copy link

Description

While editing the site in full screen and distraction free mode, the header is empty when hovered.

This is only happening in the site editor. Post editor is behaving as expected.

I bisected the behavior change to have been introduced in 3ece636

Step-by-step reproduction instructions

  1. Activate Twenty Twenty Four
  2. No other plugins
  3. Checkout and build commit 3ece636
  4. Visit the site editor /wp-admin/site-editor.php?canvas=edit
  5. Drag the sidebar away (full-screen mode)
  6. Turn on "distraction free" mode
  7. Hover over header

Then you can re-build with the parent commit a7a62f7, and it should work as expected.

Screenshots, screen recording, code snippet

Screen.Recording.2024-04-18.at.12.20.45.PM.mov

Environment info

  • WordPress 6.5.2 stable
  • Chrome Version 123.0.6312.124 (Official Build) (arm64)
  • Gutenberg trunk

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@dereksmart dereksmart added the [Type] Bug An existing feature does not function as intended label Apr 18, 2024
@Mamaduka Mamaduka added [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Feature] Distraction Free A preference in the Post and Site Editor that limits distractions to focus the editing experience labels Apr 18, 2024
@Mamaduka
Copy link
Member

cc @youknowriad, @draganescu

@youknowriad
Copy link
Contributor

Thanks for catching this. I tried looking and it's a bit challenging. The distraction free "hover" animation is split into two parts:

  • the first hover event handler animates the hub (so the hub needs to be 100% wide)
  • the second hover event handler animates the header itself

The problem now is that the header is not rendered within the hub anymore, the hover event of the header is not triggered anymore. It seems there's no way to trigger two hover events for elements that are not within each other (one covering the other).

I see two options:

  • A hack: simulate a hover event manually on the header when hovering the hub (dispatchEvent)
  • A better fix but requires some design changes: Render the hub within the header (rather than keep it on top of the header but visually make it look like it's in the header). The animation when entering and exiting the "edit" mode for the hub becomes harder to achieve.

@draganescu
Copy link
Contributor

@jeryj I think this is worth a look from you as you've been improving this animation a bit in the past as well. Maybe you have more to add to what Riad described above.

@jeryj
Copy link
Contributor

jeryj commented Apr 19, 2024

I found a way to share the header animation state via a context and using that variant name to set the animation of the other components. It's far from finished, but it may be a way forward with enough tweaking to be able to use one div to set the animation variant and pass it to the others: #60916

What do you think @youknowriad?

@youknowriad
Copy link
Contributor

For me it's still a hack. The interface skeleton's API has been changed to receive a variant prop for the header? It doesn't make much sense in terms of API for that component.

I'm of the opinion that we should do my proposed hack 1 temporarily and try to implement 2 longer term. In the post editor the "toggle" (hub) or whatever is part of the header, it should be the same in the site editor.

@jeryj
Copy link
Contributor

jeryj commented Apr 19, 2024

The interface skeleton's API has been changed to receive a variant prop for the header? It doesn't make much sense in terms of API for that component.

It's less about the API of how things are passed around. I wasn't focused on that. I more mean the design of using consistent variant names and using those variant names to keep all the animation states in line. So, for your recommended hack:

A hack: simulate a hover event manually on the header when hovering the hub (dispatchEvent)

Rather than simulate a hover event manually on the header, set an animation state of a variant name somewhere that can be read by whatever wants to use it.

@youknowriad
Copy link
Contributor

Rather than simulate a hover event manually on the header, set an animation state of a variant name somewhere that can be read by whatever wants to use it.

Yeah I understand and could be fine as a temporary solution but I just don't see why the header animation need to be set on the "layout" hub component. Why would the "layout" of the site editor pass a variation name to the header of a specific page/route of the site editor. The layout shouldn't even know about the registered routes/pages. Ultimately the "shell" of the site editor and the routes/pages will be in completely different packages probably as we'll have other pages in WP-Admin that may use the same "shell".

@jeryj
Copy link
Contributor

jeryj commented Apr 19, 2024

I just don't see why the header animation need to be set on the "layout" hub component.

It wouldn't need to be set there. The example I showed is a proof of concept of how setting and reading a variant name might be a good way forwards so we can trigger multiple layers of animations consistently from wherever makes the most sense. I'm not committed to this approach, fwiw. Trying to provide more options that could work.

Render the hub within the header (rather than keep it on top of the header but visually make it look like it's in the header). The animation when entering and exiting the "edit" mode for the hub becomes harder to achieve.

If we go this route, we'd need to make sure the semantics of the page are correct and focus is handled between elements disappearing/appearing.

@draganescu
Copy link
Contributor

👋🏻 given that this is a shipped bug and it's also a bug that prevents action (user is stuck in distraction free, user is stuck with no save/publish) unless they visit the post editor to exit DFM, I would vote to land a hack if the proper solution is too convoluted.

@youknowriad youknowriad linked a pull request May 10, 2024 that will close this issue
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label May 10, 2024
@stokesman
Copy link
Contributor

stokesman commented May 22, 2024

In anyone subscribed here has some availability you may wish to test the hack I made at #61874 as that fixes this in my testing. I'd like to think it may not be a hack but I don’t comprehend its potential impacts.

UPDATE: Closed that PR. I don’t think we can consider it and its primary purpose wasn’t to fix this issue—it just happened to. The primary thing it meant to fix is better done by #61882 (which unfortunately didn’t impact this).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Distraction Free A preference in the Post and Site Editor that limits distractions to focus the editing experience [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
6 participants