Show UI warning if Pages cannot be retrieved in Page List block#37486
Merged
Show UI warning if Pages cannot be retrieved in Page List block#37486
Conversation
|
Size Change: +110 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
Contributor
|
I did a manual test: Single page list block: The message displays correctly when the logged in user does not have the permission. |
ba5bb0a to
90e0e78
Compare
Contributor
Author
|
@carolinan Thank you for taking time to test 🙇
That's likely because the branch wasn't rebased against |
90e0e78 to
001f6bb
Compare
Contributor
Author
|
@Mamaduka I think it would be worth including this in any post-5.9 patch release. How do I mark it as such? |
Member
|
@getdave sounds good 👍 Added the "Backport to WP Minor Release" label. |
Member
|
I have tested this locally and can confirm that it is now working as described 👍 |
fabiankaegy
approved these changes
Jan 7, 2022
Member
fabiankaegy
left a comment
There was a problem hiding this comment.
The code also looks good to me 👍
Member
|
Cherry-picked for the minor. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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
In #37368 we learnt that the Page List will show a spinner forever if you are logged in as a lower permission user (e.g. Contributor).
The reason for this is complicated and a true fix would be far outside the scope of this PR.
As a result, all we can do is show an error message to the user that Pages cannot be retrieved.
This is what this PR does. If the API returns no response (i.e.
nullrather thanundefined), we show an error to say we couldn't retrieve the Pages.Whilst far from ideal, this is somewhat better than an infinite loading state.
Why do we need to include this in 5.9
The reason I feel this is important is that Page List is used extensively by the Navigation block and also by block Themes which often use the Page List as the default state for the Navigation block (e.g. in patterns).
Currently if a lower permission user were to encounter a Nav block with a Page List inside it, they would see an infinite loading state spinner and assume that Nav block was broken when in fact if would be the Page List. This is shown in the screenshot below:
String freeze change
If we cannot include this PR in 5.9 due to string freeze, then another option would be to reuse the existing error notice which whilst not ideal would still suffice.
Closes #37368
How has this been tested?
Switch to Contributor role user.
Add Page List block.
See no infinite loading state.
See warning / error notice.
Now switch to Admin.
Create a Nav block and click
Add all Pages.Save.
Switch to Contributor.
new Post
Add Nav block.
Click
Select menuand select the menu you created as Admin.See the Page List block inserted and showing the error message and not the infinite loading state.
Screenshots
Standalone Page List
Page List inside Nav block
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).