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

Limit nick length and whitespaces in UI #344

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

pastly
Copy link
Contributor

@pastly pastly commented Feb 19, 2016

Fixes #337.

Max nick length of 24 was chosen arbitrarily.

It seemed necessary to make changes in both ContactUser.cpp and UserIdentity.cpp even though only the changes in ContactUser.cpp seemed to have any affect.

@s-rah
Copy link
Member

s-rah commented Feb 19, 2016

I think the spirit of the change is fine - however, I think 24 is too short. As a usecase, I have many ricochet contacts who I reference by name(@twitter handle) - even taking by name only just makes the cut - "Sarah (@SarahJamieLewis)" and there are a fair few of my contacts who would be truncated by this change.

With the simplification removing the troubling whitespace, the only time that length is an issue is in the "...is online/offline" display, truncating it at this UI point might be a nicer solution, without having to make arbitrary decisions about name length.

@pastly
Copy link
Contributor Author

pastly commented Mar 22, 2016

Now it only makes changes to what is displayed. In this order:

  • repeated whitespace characters are squashed into a single space
  • leading and trailing whitespace is removed
  • truncated to the first 80 characters

@pastly pastly changed the title Limit nick length and whitespaces in ContactUser and UserIdentity Limit nick length and whitespaces in UI Mar 22, 2016
@special
Copy link
Member

special commented Apr 10, 2016

These limits sound reasonable.

We shouldn't be changing the nickname at display-time, though. We'll end up having to copy this code to a lot of different places, and the behavior will be unexpected for some cases.

Can you apply this when setting the nickname for a contact, instead?

@pastly
Copy link
Contributor Author

pastly commented Apr 19, 2016

Yes. So back to silently truncating, but at 80 characters instead and additonally trimming and replacing repeated whitespaces with single spaces?

@special
Copy link
Member

special commented Apr 19, 2016

Sounds good.

Ideally, we should also change the UI where nicknames is set, to have the same maximum length and avoid confusing users too much.

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

Successfully merging this pull request may close these issues.

None yet

3 participants