Avatar block: error on loading if Avatars are disabled on Settings#41577
Closed
cbravobernal wants to merge 2 commits into
Closed
Avatar block: error on loading if Avatars are disabled on Settings#41577cbravobernal wants to merge 2 commits into
cbravobernal wants to merge 2 commits into
Conversation
cbravobernal
added
[Block] Avatar
|
Size Change: +4 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
Contributor
Can we add a warning, for consistency with what we're doing in the Post Comments block (and in some Comments blocks)? FWIW, I've recently discussed this subject with @jameskoster, and we kind of settled on warnings after all. |
Contributor
Author
|
Hi @ockham , I'm going to move the warning to a new issue/PR, as will need some design feedback. As we can have Avatars anywhere on the site, the warning does not fit in all places (like the header, each comment, etc)
|
Contributor
|
I wonder if we could place the errors in List View instead 🤔 |
Contributor
Author
Let's do that experiment in another PR, so we can fix this error in the next minor. Closing this PR as it will be fixed in #41354 |
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.
What?
Fixes #41308
Closes #41313
Closes #41354
Why?
Right now, if Avatars are disabled on settings and we try to add it as a block, we got an error:
How?
Maybe it would be a good idea to make this block conditional if Avatar is disabled, or add a warning. But in this PR, we are just avoiding the JS error (the avatar will load the default one on the editor, and not in the frontend if they are disabled, causing some inconsistency between them).
We can maybe address it in another PR after some design feedback:
cc @javierarce , @richtabor, @estelaris
Testing Instructions
In trunk:
1.) Disable avatars on settings.
2.) Add Avatar block.
3.) Check the crash
After the fix:
1.) Disable avatars on settings.
2.) Add Avatar block.
3.) Avatar is displayed on the editor, not in the frontend.
Screenshots or screencast