-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add warning when username starts with a lowercase letter #194
Add warning when username starts with a lowercase letter #194
Conversation
Warning for people who select a username starting with a lowercase letter
Closes #193 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good by looking at the code, will validate when I get the chance.
Co-authored-by: AbyxDev <[email protected]>
resources/main.js
Outdated
|
||
elem.onblur = function() { | ||
var currentname = elem.value || ""; | ||
var usernameblock = new OO.ui.infuse(elem.parentElement.parentElement.parentElement.parentElement); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to use .closest
instead of this? This seems janky and likely to break with some OOUI update.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the one issue I mentioned about repeated calls to .parentElement
this looks good. Please look into using .closest
for a more robust solution or let me know if you can't find anything.
@jacob-g I'm not familiar with .closest but I don't think it's likely to break. Generally the only thing that can break is hardcoding element names (like "ooui-php-1"), but just the number of elements is fine. I will look into .closest more if it's important. |
You can use this: document.getElementsByName("scratchusername")[0].closest('.oo-ui-layout') This both is less likely to break if OOUI introduces any change and more properly conveys the abstraction of trying to find the wrapping layout. |
@jacob-g OK. Added but not tested. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good now.
Warning for people who select a username starting with a lowercase letter