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

Layout shift on page load #183

Closed
johnhooks opened this issue Mar 25, 2023 · 7 comments
Closed

Layout shift on page load #183

johnhooks opened this issue Mar 25, 2023 · 7 comments
Labels
bug Something isn't working [Scope] User Interface For displaying to and interacting with end users.
Projects

Comments

@johnhooks
Copy link
Collaborator

johnhooks commented Mar 25, 2023

Description

Every page transition in the admin area cause a layout shift when the notifications render.

Step-by-step reproduction instructions

  1. Transition between pages in the admin area.

Screenshots, screen recording, code snippet

layout-shift

Environment info

  • latest develop branch b1519c8
  • npx wp-env start

Possible Solutions

Because React cannot be rendered on the server, I suggest we consider using Dynamic Blocks for the UI components. This would provide SSR (unsure how client hydration can/does work, will need more research). We would be using blocks outside the editor, but this is what is planned for the new Admin area, as detailed in this post: Thinking Through the WordPress Admin Experience.

A good place to experiment would be the dashboard, which causes the layout shift issue. The Notification Hub (sidebar) does not cause the layout shift issue when client rendered. While it would be interesting to remember the state of the sidebar and server render it in the appropriate state, the idea should be saved for a later iteration, if experiments with Dynamic Blocks are successful.

Revision 1

After receiving advice, I don't believe my initial proposal will solve the problem. Currently there isn't a good solution to hydrate blocks with React outside the editor. Work is being done to accomplish this, but it is very experimental.

Revision 2

This could be promising. Keep an eye on how it matures The Interactivity API – A better developer experience in building interactive blocks

@Sephsekla Sephsekla added bug Something isn't working [Scope] User Interface For displaying to and interacting with end users. labels Mar 27, 2023
@Sephsekla Sephsekla added this to To do in Admin UI Mar 27, 2023
@johnhooks
Copy link
Collaborator Author

johnhooks commented Mar 30, 2023

There was a discussion in the Slack #feature-notifications channel about this problem.

My summery of that conversion.

Dynamic block rendering of dashboard messages is a reasonable concept. The serialized block should not be saved to the database, but we can try the method of the server side render_callback to render a block and then pick it up on the client and add interactivity. Whether or not this is advantageous over tradition PHP rendering is to be determined.

After some discussion in WordPress/gutenberg#38224, I'm unsure whether or not we can reliably hydrate the block in the client. The ability to apply a filter to the rendered content of a block in SSR means the HTML structure could be modified and not hydrate correctly.

@johnhooks
Copy link
Collaborator Author

johnhooks commented Mar 30, 2023

Another option is to not use React, but use a combination of traditional PHP and plain JS. It would still require duplication. But I'm learning there currently isn't a good solution in WordPress to accomplish a seamless SSR to client hydration using React. The Admin area is still a traditional PHP rendered application, trying to layer an SPA over the top creates a disjointed experience.

The Hub is less problematic than the dashboard because it is out of the normal page flow. But it will still either:

  • Constantly flicker open and close on every transition, if the open state is remembered.
  • Constantly close on every transition and the user will feel like they have to keep opening it.

@johnhooks
Copy link
Collaborator Author

We know on the server whether or not a user has notices, we can render out fillers on the server with loading states. Then we don't have to ssr the actual notices, just something that fills their space. And it will be replaced when the actual notices are loaded, using something like this https://github.com/zalog/placeholder-loading.

For this to work the notices would need to have a fixed height. And larger notification messages would need to need to be shorted with an excerpt which is expanded by the user.

The data can also be enqueued as a script to be loaded with the html, so we don't have to fetch the first set of notifications, they can be picked up immediately.

@erikyo
Copy link
Collaborator

erikyo commented Apr 9, 2023

I think there are some difficulties in doing this... for example we would have to know in advance how many notifications are in the dashboard and that would force us to do a database query. And I think that's exactly what we'd like to avoid because, at least IMHO, the plugin shouldn't block the execution and run these subtasks afterwards.
Also, as you point out, if we want to use placeholder to avoid shifting we have to set a fixed height for the notifications, which is something we cannot guarantee (or for the sake of customization we don't want to guarantee).

So I propose that if we wanted to "pre-"know the number of notifications we might query the entire array of notifications at this point, since I don't think there is much difference in term of ms (and we will also avoid a double query).

Differently (which maybe I would prefer) it would be better to use a different way to display notifications...

shifting

The shifting issue is already present in all the banners/notifications at the top of WordPress admin area and it is simply due to the fact that they are added to the top as a "block" with "static" position while if they were floating (e.g. position absolute/fixed) this problem would not happen

@johnhooks
Copy link
Collaborator Author

Rendering outside the normal page flow would be a great solution. Providing a way to indicate a new notice has been added to the Hub, like a floating notification in the bottom right that hides after a delay.

A modal context could be a good option for urgent alerts, especially when user input is required.

@EdithAllison
Copy link
Collaborator

@Sephsekla This looks superseded by the design discussion in #357. I am suggesting to close this issue & continue discussion based on new Figma designs. As the new design uses a modal, there's no layout shift.

@johnhooks
Copy link
Collaborator Author

@EdithAllison that sounds good to me, I will close this one.

Admin UI automation moved this from To do to Done Aug 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working [Scope] User Interface For displaying to and interacting with end users.
Projects
Development

No branches or pull requests

4 participants