Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New room list: Action local echo #14302

Open
turt2live opened this issue Jul 2, 2020 · 19 comments
Open

New room list: Action local echo #14302

turt2live opened this issue Jul 2, 2020 · 19 comments
Labels
A-Room-List O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience T-Enhancement Z-Papercuts Visible. Impactful. Predictable to action. Z-TravisR

Comments

@turt2live
Copy link
Member

Leaving, tag changes, etc

@turt2live turt2live added this to TODO in Room List v2 via automation Jul 2, 2020
@turt2live turt2live moved this from TODO to Nice to haves (time permitting) in Room List v2 Jul 2, 2020
@turt2live turt2live moved this from Distant future to Dev TODO in Room List v2 Jul 16, 2020
@turt2live turt2live moved this from Dev TODO to TBD Bug Queue in Room List v2 Jul 20, 2020
@turt2live turt2live moved this from TBD Bug Queue to Now (ready for dev) in Room List v2 Jul 20, 2020
@t3chguy t3chguy moved this from Now (ready for dev) to Soon (queued for design) in Room List v2 Jul 22, 2020
@t3chguy
Copy link
Member

t3chguy commented Jul 22, 2020

Need design for how to handle the request failing, just undoing the change would cause user confusion, need a contextual/modal error interaction

@niquewoodhouse
Copy link

I agree, just undoing would be pretty confusing and frustrating. Some quick thoughts:

  • Local echo keeps trying to send
  • If errors after 60 (or XX) seconds, show them the error.
  • Question: would we be able to keep trying endlessly, is there a need for someone to do

A global toast feels like less than ideal because it's so distant from the context of what the user is trying to do.
error@2x

A local info/error icon, that can explain more
error-2

Early thoughts on these or other suggestions would be very welcome!

@t3chguy
Copy link
Member

t3chguy commented Jul 23, 2020

would we be able to keep trying endlessly

We could keep retrying but at that point the user's context to the action they took however many minutes ago is lost and the change wouldn't have reflected on the device.

There are many reasons why it could go wrong:

  • Server says no

  • Server outage

  • Internet outage

  • TLS Error

  • CORS Error (could be a timeout)

  • Local Firewall/AV

  • Browser Extension

We can only tell the difference between Server says no and the rest of them

@niquewoodhouse
Copy link

@t3chguy Thanks for putting a list of reasons together. So essentially, whether it's a global toast or something contextual to the room, we need two copy variants ("server says no" "other") for each scenario? I think it might be helpful for users, in that copy, to have access to a page like "Server issues FAQ" or something that details these reasons, with possible things they could do (eg "just leave it for a bit/check internet connection/etc"). Or they could just assume Element is failing them.

Screenshot 2020-07-24 at 13 59 57

Do you think it's reasonable to expect that if the server fails to do one thing (eg change notifications from default > mentions & keywords), it's probably going to be failing to do a few things? I wonder if that makes the global toast approach super annoying because there will be XX toasts appearing that you need to cycle through to even be able to search/log out.

And something smaller contextual to each action is better?
error-3@2x

Does that sound better to you?
Does it sound feasible in practice?

So far I've proposed 3 things and I'm not sure I know enough to be confident of what's the best solution out of them/if any are good. I think you (@t3chguy and @turt2live) have much more knowledge here so I'd be super interested in knowing your perspective.

  • Toast use
  • Info icon in room list with pop up info
  • Contextual thing to each action

Do we have a rough expectation for how often these errors could occur? If they're expected to be an edge case, maybe the first iteration is using what we already have - eg the toast - and evolving from there.

Even though I'm not convinced it's the best solution it might make sense, especially if we'll have a global notifications thing later, where it would probably make sense to include this kind of thing.

Thanks for the help

@t3chguy
Copy link
Member

t3chguy commented Jul 24, 2020

it's probably going to be failing to do a few things?

Possibly but in the case of large deployments like matrix.org there are separate Synapse workers doing various things so this gets much less likely here.

Does it sound feasible in practice?

Sure but what if the user already closed the Context Menu, such as when using the keyboard; the accessible interaction means that if you're using keyboard and use Enter it'll close the context menu as per the WAI-ARIA guide so you'd never see that contextual error.

@niquewoodhouse
Copy link

Sure but what if the user already closed the Context Menu, such as when using the keyboard; the accessible interaction means that if you're using keyboard and use Enter it'll close the context menu as per the WAI-ARIA guide so you'd never see that contextual error.

I was thinking that you'd close the menu, you wouldn't see the contextual error & you get on with whatever else you're doing. In the bg, we're trying to send to the server. If after a while you realise the change hasn't happened you'd re-open the menu and because we're still trying to send something to the server, this would be injected into the context menu. It's more subtle than "something is broken/wrong" being said globally, and its got the context from being grouped with the action.

@t3chguy
Copy link
Member

t3chguy commented Jul 24, 2020

If after a while you realise the change hasn't happened you'd re-open the menu and because we're still trying to send something to the server, this would be injected into the context menu.

This would be functionally hard to implement but possible

How would one clear the error in that case?
Which state would be shown in the radio interaction, the old one or the one which failed to be set?

@niquewoodhouse
Copy link

Thanks for investigating these suggestions @t3chguy 👍 I think if something is going to be hard to implement its probably not the best option, especially if this is an edge case that ideally won't happen.

How would one clear the error in that case?

You can't. The error is caused by the latest choice not making it to the server. So it's there until you make a choice that is successfully sent to the server.

Which state would be shown in the radio interaction, the old one or the one which failed to be set?

The old one. The old one is what it is true, because the server hasn't responded to the fail. Its like when you try to save a G-Doc file, but you're offline. Its useful for the user to know what it is so they can decide to retry their action. Retry > sends to server > error disappears. Retry > can't send to server > old one is shown as active, because it is & error state is shown.

Although I think this might be actually helpful, it all sounds a bit complicated and I don't think it's wise to add much complication.

Here is one more suggestion, based a bit on what I'm learning from your comments. These are rough mockups but hopefully good enough to illustrate the idea.

Global non-toast reference to "status"
error-4@2x

error-5@2x

error-4b@2x

These keep the global-ness so we can avoid a bit of complexity. The placement next to the user name will be phased out when we do the notification center, which might be the right place for this long-term anyway.

@t3chguy
Copy link
Member

t3chguy commented Jul 24, 2020

Those global things seem too ambiguous, what if a server is only partially out or just rejects the notifications change but works fine otherwise. Also it doesn't say what actions failed which seems like it'll just fuel confusion

@niquewoodhouse
Copy link

Also it doesn't say what actions failed which seems like it'll just fuel confusion

@t3chguy If they said what actions failed in some way does this become a good option to you?

Those global things seem too ambiguous, what if a server is only partially out or just rejects the notifications change but works fine otherwise.

Well, it could be that it has a few levels to it, like a wifi signal. This feels a bit convoluted but maybe it helps that make sense.

My concerns with the localised/contextual ones are:

  • they'll mean lots of work and future product maintenance
  • you could end up with a UI like the previous room list, where there's a load of red encryption errors that make the whole thing heavy and much harder to use

I can understand them for things like typing in a room where it says "server is unavailable" as that's core to the experience, but maybe having one for each individual thing means it can become complicated easily. With one error/status notice somewhere at least I can still get on with using Element.

@t3chguy
Copy link
Member

t3chguy commented Jul 24, 2020

good option to you?

In the end it isn't about whether it is a good option for me as I am far from a typical user, I am trying to raise points to ensure you know all angles of this as they come up so that the solution that you come up with considers them :)

@niquewoodhouse
Copy link

Is there somewhere where we have a list of the 20 most likely things that will use local echo? Maybe @turt2live or @t3chguy or @nadonomy have this?

I just want to be able to stress-test the options (what does it look like if 5/10/20 things didn't get to the server) to work out which is the safest option.

Thanks

@t3chguy
Copy link
Member

t3chguy commented Jul 28, 2020

So basically every synchronized setting should have local echo (most don't today though)

Pretty much every setting under Room Settings could have local echo
The vast majority of User Settings could too.
Obviously things with an explicit Save button in both shouldn't.

The one thing we have solid local echo with is the timeline, when you send a message you get a greyed message in the timeline and it goes white when succeeds or you get an error at the bottom of your timeline if it fails.

@niquewoodhouse
Copy link

After all the above discussion and looking at the options with the design team, we'll go for a global notice, that isn't the pre-existing toast.

Reasoning:
If there are many things that could have local echo we should have a UI that scales easily and doesn't require everything needing a custom element inserted, which will just cause extra work at every turn and thus slow us down from shipping improvements.

Example:
Kapture 2020-07-29 at 13 10 45

The elements are

  • an "offline mode" badge/notice (which may have a tooltip if it is just an icon)
  • a modal that has more information within it

As there's a need to get this shipped ASAP, I have some variants of the global notice that I'm happy with as a first iteration, and would be open to the one used being the simplest to make. The options are this artboard and the ones to the right of it.

I'm open to feedback that helps refine the copy and confirm which placement of the badge/notice would be most feasible for the next release as we can iterate afterwards if necessary.

@turt2live turt2live moved this from Soon (queued for design) to Now (ready for dev) in Room List v2 Jul 29, 2020
@turt2live turt2live self-assigned this Jul 29, 2020
@turt2live turt2live moved this from Now (ready for dev) to In progress in Room List v2 Jul 29, 2020
@turt2live turt2live added this to In Progress in Web App Team via automation Jul 29, 2020
@turt2live
Copy link
Member Author

ftr the one linked seems to be easiest/fastest, though the icon in the top left by the username feels better for a future iteration. Personally I'm a fan of the "Offline mode enabled" text, as it means we can wrap the "Your server is offline" angry notice into it.

For those who can't see the thing:
image

@turt2live
Copy link
Member Author

Have opened #14818 to track future iteration

@jryans
Copy link
Collaborator

jryans commented Aug 5, 2020

@turt2live What tasks are remaining on this issue?

@turt2live
Copy link
Member Author

At least leaving rooms, tag changes, etc.

@turt2live
Copy link
Member Author

Going to drop this off the board for now as it's not entirely in progress.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Room-List O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience T-Enhancement Z-Papercuts Visible. Impactful. Predictable to action. Z-TravisR
Projects
Room List v2
  
In progress
Development

No branches or pull requests

7 participants