Site Editor: Check whether the data is loaded. If the data is loaded and the template is still unknown, give appropriate feedback #30664
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
|
Size Change: +136 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
aristath
approved these changes
Apr 12, 2021
|
That PR is fine but I wonder if it's actually better to show a message on a higher level (hide the entire page) similar to what is done on the post editor?
|
|
Hmmm I kinda prefer this implementation... It still shows the site-editor and I can navigate to a different template and keep working. The post-editor one is good at what it does, but it would feel a bit like a screen-of-death in the site-editor IMO 🤔 |
|
Maybe then a message on the "canvas" of the site editor? I don't feel strongly here either way :) |
youknowriad
added
[Feature] Full Site Editing
[Feature] Site Editor
This. The text in the header doesn't really stand out in this case so it's easy to miss it. We could use the same message as we use for the post editor: "You attempted to edit an item that doesn't exist. Perhaps it was deleted?" Otherwise looks good and works well! Thanks for working on this! 🙇 |
|
Thank you both for the feedback, excellent points! @bobbingwide is currently unavailable to continue working on this (the PR was part of a contributors-day) so I pushed a commit to add a notice in the canvas when the template can't be found:
Would that work? |
I think you meant @boblinthorst |
|
Oops, my bad... apologies, bad github auto-complete 🤦 |
|
Looks good to me! Once we have a test I'm happy to approve 😄 |
That's going to be a problem... I don't (yet) know how to write these tests 🤐😅 |
Would you be interested in writing your first test? :D If not, let me know and I'll take it over. First of all, we don't really have a function that you can use to visit the site editor with specific URL parameters. In our case we need to visit the site editor with custom URL parameters to access a non-existing template. You can extend the existing query.
Once that's done you can create a new test file in: https://github.com/WordPress/gutenberg/tree/trunk/packages/e2e-tests/specs/experiments You should copy-paste an existing test as a base for your test. https://github.com/WordPress/gutenberg/blob/37e15797f2259ee0661b56b5a8f877bde9f29099/packages/e2e-tests/specs/experiments/site-editor-export.test.js is a very short test file that should work for you. |
…ate is still unknown, give appropriate feedback.
david-szabo97
force-pushed
the
29265-give-feedback-when-template-nonexisting
branch
from
7b26528 to
4d7d668
Compare
david-szabo97
approved these changes
Apr 30, 2021
david-szabo97
changed the title
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.
Description
Created to try and resolve #29265.
Shows "Template not found" if there's no found template for the post id.
I'm not particularly comfortable in this context, so if this PR does not solve the issue in the desired way, don't feel bad about closing it.
How has this been tested?
I visited the proved link in the issue, and checked the added feedback is shown. I also visited a normal template page, to ensure this feedback does not show when it is not appropriate.
Screenshots
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).