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

Picking a username in ILAG with a capital letter fails unpleasantly #5833

Closed
ara4n opened this issue Dec 13, 2017 · 10 comments
Closed

Picking a username in ILAG with a capital letter fails unpleasantly #5833

ara4n opened this issue Dec 13, 2017 · 10 comments
Assignees
Labels
P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@ara4n
Copy link
Member

ara4n commented Dec 13, 2017

You get a nasty error telling you to change the username to be lowercase in a very cryptic manner. I thought we squashed case to lowercase on the server at registration these days?

@ara4n
Copy link
Member Author

ara4n commented Dec 13, 2017

image

@pafcu
Copy link
Contributor

pafcu commented Dec 14, 2017

Related: #5434

@lampholder
Copy link
Member

lampholder commented Jan 11, 2018

http://jsfiddle.net/5ocu2cqk/20/ for the win!

We should really have microcopy on these fields too, to explain what how a username is used and to lay out the restrictions before a user starts trying to type capitals/Chinese/punctuation/whatever

@lampholder lampholder added T-Defect ui/ux P2 S-Major Severely degrades major functionality or product features, with no satisfactory workaround labels Jan 11, 2018
@lukebarnard1
Copy link
Contributor

lukebarnard1 commented Feb 22, 2018

Yep, I'm with @lampholder on this one - we already automatically set the display name to the local part, let's kill two birds here and use the fiddle but word it for the user.

I suppose we could handle characters outside of [a-z0-9=_-./] by replacing them with some character? Maybe "-" or "_"?

@turt2live
Copy link
Member

With /app this is slightly different now: It automatically lowercases your username for you and happily tries to register it, however the bright red warning about invalid characters is still there.

@lukebarnard1
Copy link
Contributor

It automatically lowercases your username for you

We definitely need to feed this back to the user before they register myusernameisawesome when they thought they were getting MyUsernameIsAWESOME.

The solution above does this.

@ara4n ara4n added P1 and removed P2 labels Sep 11, 2018
@turt2live
Copy link
Member

@lampholder is cloning the fiddle in #5833 (comment) the expected outcome for this issue?

@turt2live turt2live added the X-Needs-Info This issue is blocked awaiting information from the reporter label Dec 7, 2018
@lampholder
Copy link
Member

I loved this at the time, now I think that there's probably a generally accepted best practice for field validation that we shoudl follow rather than (potentially) surprising people with their keypresses not appearing.

Let's get a designer's opinion: @nadonomy, what do you think?

@turt2live turt2live added design X-Needs-Design and removed X-Needs-Info This issue is blocked awaiting information from the reporter labels Dec 7, 2018
@nadonomy
Copy link
Contributor

nadonomy commented Dec 7, 2018

We should definitely avoid swallowing characters (as per the jsfiddle) and silently modifying case server side (to avoid situations like this).

Looking at the current ILAG behaviour on /develop there's a few tweaks we can make to make this feel better:

  • Make the error message way more verbose:
    • Only use lower case letters, numbers and '=_-./'
    • (instead of: Username invalid: User ID can only contain characters a-z, 0-9, or '=_-./')
  • Stop the modal jiggling size when validating input
    • Instead of using the spinner, I'd just update the feedback underneath to display 'Checking...' in the normal body text colour; your eye knows to expect to see stuff there
  • Make the error messages consistent
    • I assume they're coming from different places (client/server?) but e.g. entering # yields 'User names may only contain letters, numbers, dots, hyphens and underscores.' instead of the error message above

@turt2live turt2live self-assigned this Dec 13, 2018
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Dec 13, 2018
Fixes element-hq/element-web#5833

This also includes changing some Jira references that aren't searchable anymore, and a thing to replace the spinner on the SetMxidDialog as per element-hq/element-web#5833 (comment)
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Dec 13, 2018
Fixes element-hq/element-web#5833

This also includes changing some Jira references that aren't searchable anymore, and a thing to replace the spinner on the SetMxidDialog as per element-hq/element-web#5833 (comment)
turt2live added a commit to matrix-org/matrix-react-sdk that referenced this issue Dec 13, 2018
Fixes element-hq/element-web#5833

This also includes changing some Jira references that aren't searchable anymore, and a thing to replace the spinner on the SetMxidDialog as per element-hq/element-web#5833 (comment)
@turt2live
Copy link
Member

Fixed by matrix-org/matrix-react-sdk#2351

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Development

No branches or pull requests

6 participants