-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Template Part: Avoid button layout shift #33362
Conversation
Size Change: +43 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
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.
Tested and this works as described. This should fix the Issue whereby the layout shifts underneath the user which is pretty frustrating for a human and also bad for writing robust e2e tests.
The only thing I wonder about is whether there should be pattern whereby if the network requests takes too long to resolve we show the "New" button early even if the Choose existing hasn't resolved.
Overall however, I think this should ship as it is an improvement.
Thanks for working on this!
This looks great, thank you for the fix! |
Description
PR adds a simple loading state while template part replacements are loaded to avoid button layout shift. The layout shift was causing the "Multi-Entity Saving" e2e test to fail from time to time.
See #33169 for complete details.
How has this been tested?
Screenshots
CleanShot.2021-07-12.at.17.19.34.mp4
Types of changes
Bugfix
Checklist:
*.native.js
files for terms that need renaming or removal).