matrix-react-sdk icon indicating copy to clipboard operation
matrix-react-sdk copied to clipboard

Improve UX around decryption errors

Open duxovni opened this issue 2 years ago • 5 comments

  • Get rid of the **ugly text** and red shields on every single message
  • Add a single banner at the top of the room view explaining what's wrong, and offering a clear call to action when possible
  • Send key requests automatically instead of prompting the user to click a link/button
  • Prompt the user to verify their device as a first step if it isn't verified yet

Screenshots

New decryption error UI

Show a loading spinner for the first 5 seconds

20220608_00h34m10s_grim-cropped

If this device isn't verified, and there are other devices or backups that could help it decrypt messages, prompt to verify first

20220608_00h37m11s_grim-cropped

If this device is verified and there are other verified devices, prompt to open one of them

20220608_00h37m55s_grim-cropped

If this is the only verified device, not much the user can do, so just explain the current status

20220608_00h37m25s_grim-cropped

If this device is unverified, and there's nothing for it to verify against, all we can do is reset keys

20220608_00h35m37s_grim-cropped

Dark mode, message bubbles

20220608_00h38m17s_grim-cropped 20220608_00h38m31s_grim-cropped 20220608_00h38m55s_grim-cropped


Here's what your changelog entry will look like:

✨ Features

  • Improve UX around decryption errors (#8272). Contributed by @duxovni.

Preview: https://pr8272--matrix-react-sdk.netlify.app ⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

duxovni avatar Apr 09 '22 04:04 duxovni

Codecov Report

Merging #8272 (75e0ce9) into develop (bca9caa) will decrease coverage by 0.01%. The diff coverage is 28.73%.

:exclamation: Current head 75e0ce9 differs from pull request most recent head 2d4969b. Consider uploading reports for the commit 2d4969b to get more accurate results

@@             Coverage Diff             @@
##           develop    #8272      +/-   ##
===========================================
- Coverage    30.05%   30.03%   -0.02%     
===========================================
  Files          881      883       +2     
  Lines        50240    50301      +61     
  Branches     12799    12802       +3     
===========================================
+ Hits         15099    15108       +9     
- Misses       35141    35193      +52     
Impacted Files Coverage Δ
src/components/structures/RoomView.tsx 29.46% <0.00%> (-0.19%) :arrow_down:
...omponents/views/messages/DecryptionFailureBody.tsx 0.00% <0.00%> (ø)
src/components/views/rooms/EventTile.tsx 42.91% <0.00%> (-0.41%) :arrow_down:
src/components/views/settings/DevicesPanel.tsx 0.00% <0.00%> (ø)
...rc/components/views/rooms/DecryptionFailureBar.tsx 1.88% <1.88%> (ø)
src/components/views/messages/MessageEvent.tsx 61.81% <66.66%> (-0.45%) :arrow_down:
src/DecryptionFailureTracker.ts 71.84% <100.00%> (+5.17%) :arrow_up:

codecov[bot] avatar Apr 19 '22 15:04 codecov[bot]

It's looking great, thanks for working on this. I am proposing a few visual changes to make the errors clearer.

  1. Consider putting the banner msessage in a headline body format over two lines so it's easier to read and understand the errors. Here is an example: Screenshot 2022-05-18 at 2 13 00 pm Screenshot 2022-05-18 at 2 17 31 pm Screenshot 2022-05-18 at 2 17 16 pm

  2. Consider changing the hidden message to a text error. This will avoid the used from mistaking this for a spoiler and the treatment will look better when bubbles are activated. Sharing examples below for modern and bubbles layout: Screenshot 2022-05-18 at 2 14 30 pm Screenshot 2022-05-18 at 2 14 41 pm

  3. In our previous discussions we had also discussed showing CTAs if necessary eg. 'Start Verifictaion' if the device is not verified and 'Retry' or 'Show verified devices' incase the device was verified. Is the plan to add those CTAs to the banner?

Sharing Figma file with all the proposed changes Link.

Thank you!

amshakal avatar May 18 '22 13:05 amshakal

@amshakal Thanks for the feedback! Did you see the updated screenshots in my comments at the top of this thread? It's already got calls to action and text-based error messages.

I got rid of the asterisks because it seemed stylistically bad, but I can put them back if you think it's an improvement; instead, I added grey key icons to distinguish those messages as errors without overwhelming the user with too much red. For the text of those messages, I went with "unable to decrypt message" rather than "waiting to decrypt message", because it's not necessarily clear what we'd be "waiting" for, and in some cases there might not be anything to wait for at all. Does that sound ok?

The multi-line banner styling is a great idea, I'll get working on that. Thank you!

duxovni avatar May 18 '22 14:05 duxovni

All sounds good to me!

amshakal avatar May 27 '22 11:05 amshakal

It's looking great. Thanks @duxovni 😃

amshakal avatar Jun 08 '22 13:06 amshakal

Closing in favor of #9544.

duxovni avatar Nov 04 '22 06:11 duxovni