[Site Editor]: Fix multiple templates creation while in the process of creating one #44146
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
ntsekouras
added
[Type] Bug
ntsekouras
requested review from
mcsf,
talldan,
jameskoster,
jorgefilipecosta and
annezazu
|
Size Change: +815 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
ntsekouras
force-pushed
the
fix/multiple-templates-created-bug
branch
from
97c883d to
f020d44
Compare
|
Nice work. I don't think the Snackbar is the correct approach though, for a couple of reasons:
As an alternative could we disable the menu / modal, with an overlay and spinner?
If it's easier, a full-screen overlay / spinner applied in both cases could work too. |
|
|
@jameskoster I added overlays where needed. If we're in a Modal flow the overlay is also shown in the DropDown as a user could click outside during creation and click other menu items. |
ntsekouras
commented
Sep 14, 2022
| false | ||
| ); | ||
| } finally { | ||
| setIsBusy( false ); |
There was a problem hiding this comment.
This is an unrelated small bug I discovered while testing and throwing error.
ntsekouras
commented
Sep 14, 2022
| </MenuGroup> | ||
| </NavigableMenu> | ||
| <> | ||
| { isCreatingTemplate && <CreatingTemplateOverlay /> } |
There was a problem hiding this comment.
This is the only addition here in the DropdownMenu. The rest is whitespace.
|
Thanks Nik, I think that's better. There is one issue that I didn't anticipate though (sorry)... On a very slow connection, after clicking to create a new template, you can close the modal + click to edit a different template, all before the template has been created. template.mp4Perhaps a full-screen overlay would be better after all? I'm also wondering if the work being done in #42525 might overlap here? Edit: Do we need to do something similar with deletion as well? On a slow connection you can repeatedly delete the same template before the deletion actually takes place 🙈 |
|
Thank you so much for splitting this out and getting started on a fix 🙌🏼 +1 to needing the same for deletion. |
I don't think so, since that PR handles the loading state before rendering blocks. --cc @tyxla I just pushed some changes to try the fullscreen overlay for the template creation for now. |
ntsekouras
force-pushed
the
fix/multiple-templates-created-bug
branch
from
d3c6738 to
e175fc0
Compare
|
Yeah this is feeling pretty good now. I'd say it's in a spot to land and refine. A tiny nitpick... would it be possible to have the overlay fill the entire viewport? The small sliver of UI peeking through at the top is a bit awkward.
|
jameskoster
approved these changes
Sep 15, 2022
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?
Mentioned by @annezazu here: #43877 (comment)
Currently when we're in the process of creating a template in Site Editor, a user can trigger the creation flow for multiple templates. This PR fixes that by adding an overlay with a spinner during the request to create a template.
Based on the below @jameskoster 's #44146 (comment)
Testing Instructions
Before
template.weirdness.mov
After
I'm not creating the actual templates in the below video to showcase the overlay.
Screen.Recording.2022-09-14.at.5.43.24.PM.mov