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

FTUE User lists: Accessibility and updated design behaviour #11771

Closed
turt2live opened this issue Jan 4, 2020 · 16 comments
Closed

FTUE User lists: Accessibility and updated design behaviour #11771

turt2live opened this issue Jan 4, 2020 · 16 comments

Comments

@turt2live
Copy link
Member

Broken out from #11200

@turt2live
Copy link
Member Author

Enter now behaves to select, not to submit. Highlight first suggestion to make it clear which one is about to be selected. Leftover input should be thrown away, though the improvements here will take care of trying to hint that this will happen.

@ara4n
Copy link
Member

ara4n commented Feb 9, 2020

Another facet of this is that it's impossible to invite an unrecognised mxid currently, i think? E.g. if I try to invite @matthew:example.com there's no way to pillify it or actually DM it :/

@turt2live
Copy link
Member Author

the dialog does try to protect you from yourself by applying the "this user doesn't exist" logic to the dialog. I suppose it could probably show you an option to select it if all else fails, but the invite would almost certainly not work.

@turt2live turt2live self-assigned this Feb 12, 2020
@turt2live
Copy link
Member Author

I'm actually going to put this aside to think about it more. The dialog structure doesn't really lend itself to nice accessibility, and faking it feels like a maintenance burden we probably don't want. Going to read up on various ways to achieve this properly instead of bludgeoning some code through the review queue.

@t3chguy
Copy link
Member

t3chguy commented Feb 12, 2020

The dialog structure doesn't really lend itself to nice accessibility

Why doesn't it, now that we focus-lock properly?

@turt2live
Copy link
Member Author

Because there's two expanding lists, a false editor, and an awkward DOM structure.

@ara4n
Copy link
Member

ara4n commented Feb 18, 2020

i'm a bit worried that we've shipped this, and aside from accessibility it's (for instance) impossible to invite a user whose mxid isn't recognised. Is the idea that the mxid check always works, so there will always be a pill search result option to click on to confirm? What if the target mxid doesn't want to acknowledge whether it exists?

@turt2live
Copy link
Member Author

The design called for unknown/invalid/unroutable (profile check fails) mxids to result in an error condition. With the current design at best we could give a red error saying "Failed to invite: User does not exist" - this is a lost feature from the previous dialog where we had a continue anyways option.

From what I understand, you're supposed to filter and find the person through the amazing suggestions it gives, not dump a mxid in there and hope for the best (though obviously this use case isn't excluded).

Part of the reason for the suggestions being terrible after filtering is Synapse's output is less than great, and the dialog tries to correct some of the data by balancing it with data it thinks is better and more useful to the use case above.

@turt2live
Copy link
Member Author

or in a shorter reply: yes, it's always supposed to generate a clickable option if it can be reasonably sure the user exists, by design.

@ara4n
Copy link
Member

ara4n commented Feb 18, 2020

not dump a mxid in there and hope for the best (though obviously this use case isn't excluded).

ok, how is it not excluded?

yes, it's always supposed to generate a clickable option if it can be reasonably sure the user exists

my point is that we can’t be sure if a user exists (afaik?) - so shouldn’t it let us post a raw random mxid and invite it?

@turt2live
Copy link
Member Author

You can paste user IDs and it checks them, so the use case of ignoring the suggestions and trying to find people yourself is still possible.

We have always relied on the profile endpoints returning something if the user exists. MSC1797 never gained traction to better make this work. The new design calls for hard failure instead of a "continue anyways" button - though this could be fixed by sending it back through design.

@ara4n
Copy link
Member

ara4n commented Feb 19, 2020

You can paste user IDs and it checks them, so the use case of ignoring the suggestions and trying to find people yourself is still possible.

Then we're talking about different use cases. If I try to invite a random ID which I know exists, but whose server might be down right now (e.g. @foo:example.com), the failure mode is currently:

  • I can't hit enter to pillify the mxid - it unexpectedly clears the text input field. (In practice, it has actually appended a return character and started a new line, but this is very unintuitive - especially as the field hasn't expanded to show that it's really multiline)
  • If I hit tab to pillify, it shifts focus to the 'go' button, looking like things are working.
  • If I hit the go button (either directly or by tabbing to it), it silently creates a new room... which is entirely empty apart from myself. There are no errors at all.
  • There are no errors to tell me that it considered the mxid invalid in the first place.

This is nothing to do with accessibility - it's just a plain regression from the work which went into matrix-org/matrix-react-sdk#2317.

This feels like a pretty big regression, and shouldn't be buried in an 'accessibility' bug - can we reopen #12186?

@turt2live
Copy link
Member Author

I can't hit enter to pillify the mxid - it unexpectedly clears the text input field. (In practice, it has actually appended a return character and started a new line, but this is very unintuitive - especially as the field hasn't expanded to show that it's really multiline)

#12163

also this issue here changes the Enter behaviour anyways.

If I hit tab to pillify, it shifts focus to the 'go' button, looking like things are working.

This issue changes this behaviour.

If I hit the go button (either directly or by tabbing to it), it silently creates a new room... which is entirely empty apart from myself. There are no errors at all.

This is by design: users must be converted to pills before it'll work.

There are no errors to tell me that it considered the mxid invalid in the first place.

This is technically by design too: it doesn't suggest anyone to you so you can't complete it. If you do manage to complete it, it'll tell you that the user could not be invited.


Invites to problematic servers will fail anyways, which is part of the theory behind checking the ID up front.

I don't think there's value in reopening #12186 because it doesn't sound like the dialog is being used as designed, which this issue should help course correct. I did separately find an issue regarding inviting exact matches: #12419

@turt2live
Copy link
Member Author

(also worth noting you can always /invite @alice:doesnotexist.com and get the older "invite anyways" dialog)

@turt2live
Copy link
Member Author

@ara4n after talking with Nad we've come up with #12440 as a fix for the lack of feedback (basically autocomplete and be less opinionated about who is useful to invite). If I'm still missing the point please ping me and we'll likely need to set up a call.

@turt2live turt2live changed the title FTUE User lists: Accessibility FTUE User lists: Accessibility and updated design behaviour Feb 20, 2020
@turt2live turt2live removed their assignment Feb 27, 2020
@jryans jryans added the A-Pills label Jun 23, 2020
@jryans jryans removed the story:26 label Mar 5, 2021
@turt2live
Copy link
Member Author

I'm not sure this has enough useful information for the modern era. If the invite dialog (as it's now called) is still missing this functionality, someone please open a new issue with fresh information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants