This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Improve message error states #5897
Merged
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
This moves most of them out of the context menu.
turt2live
force-pushed
the
travis/error-states
branch
from
6b1cbdc to
cc095c8
Compare
turt2live
force-pushed
the
travis/error-states
branch
from
df914b2 to
a53696f
Compare
jryans
approved these changes
Apr 22, 2021
|
Thanks also for thinking about tests, I agree it's currently hard to cover in this case. |
gaelledel
reviewed
Apr 23, 2021
There was a problem hiding this comment.
- Just as we have the button " Retry all " leading to the "sending" state with a spinner, Could we do the same for "deleting all"? So it'd be "deleting" + spinner. Once the command is accomplished, we can remove the toast entirely.
- On dark mode on the bottom toast, can the separator colour be darker?
Apart from that all good!
|
@gaelledel do we need a state for deleting? It should happen almost instantaneously every time... For the divider, how's this? |
If it happens straight away then no we don't |
gaelledel
approved these changes
Apr 26, 2021
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes element-hq/element-web#16423
Code reviewer: I would love to add tests for this, but there's a ton of logic which makes that difficult. It mostly comes from the js-sdk's built-in retry mechanisms (so messages don't fail for ages), and the fact that echo is just not something we can test for. Sorry.
Design reviewer: Testing this manually is a bit painful, so hopefully the screenshots below help. They should cover every state.
Dark theme visuals (only changed areas):
