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

Fix/daef 466 Limit and validate Wallet name max length #465

Merged

Conversation

nikolaglumac
Copy link
Contributor

@nikolaglumac nikolaglumac commented Sep 5, 2017

This PR introduces maximum length validation (and limiting) of wallet name attribute.
Wallet name maximum length is set to 40 characters.
In the scope of this PR sidebar styling was updated - until now wallet name shown sidebar items was truncated via char count (which has different results based on width of individual characters) and now we use more precise CSS method (text-oveflow: ellipsis).

TODO

  • Inject update Japanese translation for invalid wallet name error message:
"global.errors.invalidWalletName": "Requires at least 3 and at most 40 letters."

screen shot 2017-09-05 at 13 43 25

@mention-bot
Copy link

@nikolaglumac, thanks for your PR! By analyzing the history of the files in this pull request, we identified @DominikGuzei, @tomothespian and @darko-mijic to be potential reviewers.

@DominikGuzei
Copy link
Member

Will review this now

Copy link
Member

@DominikGuzei DominikGuzei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks good! 👍
There is one problem left to fix:
screenshot 2017-09-06 um 11 00 29

(the wallet name should also be truncated in the top bar, so that it doesn't reach into the sync label)

@DominikGuzei
Copy link
Member

Hiroto just sent me the translation for the error message:
ウォレット名は3文字以上40文字以内である必要があります

The direct translation to English would be Wallet name requires at least 3 and at most 40 letters which also seems to be more precise, so you could also update that 😉

@nikolaglumac
Copy link
Contributor Author

@DominikGuzei both English and Japanese translations are now updated.
Syncing label overlap will not happen in the mainnet and creating solution which doesn't overlap is to complex so it is decided we keep things as-is.

@DominikGuzei
Copy link
Member

Travis build fails due to broken mac install script … does not seem to be related to the frontend code changes in this PR.

@DominikGuzei DominikGuzei merged commit 2ff3b26 into master Sep 7, 2017
@nikolaglumac nikolaglumac deleted the fix/daef-466-limit-and-validate-wallet-name-max-length branch September 7, 2017 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants