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.

Sign up for GitHub

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

Jump to bottom

Auto suggest username #1022

Merged
merged 16 commits into from
Feb 29, 2024
Merged

Auto suggest username #1022

Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
5ac5eee
added username suggestion based on full name
balsa-asanovic Jan 28, 2024
5d3e42f
added classes for styling dropdown in username suggestions
balsa-asanovic Jan 28, 2024
bae02a2
Merge branch 'openSUSE:master' into auto-suggest-username
balsa-asanovic Feb 5, 2024
e42c4be
Merge branch 'openSUSE:master' into auto-suggest-username
balsa-asanovic Feb 11, 2024
502a230
removed trailing space from transalation
balsa-asanovic Feb 11, 2024
83bbf5b
Merge branch 'openSUSE:master' into auto-suggest-username
balsa-asanovic Feb 19, 2024
1f9badf
moved suggestUsernames method to utils.js file
balsa-asanovic Feb 19, 2024
94dce4d
created utils.js file with method for username suggestion
balsa-asanovic Feb 19, 2024
b71102d
unit tests for utils.js file
balsa-asanovic Feb 19, 2024
a5e4a59
Merge branch 'auto-suggest-username' of https://github.com/balsa-asan…
balsa-asanovic Feb 19, 2024
06a89e6
moved suggestions menu to internal component
balsa-asanovic Feb 25, 2024
11db225
updated the algorithm for suggestion to be more readable
balsa-asanovic Feb 26, 2024
3cbb674
updated tests for suggestions algorithm
balsa-asanovic Feb 26, 2024
b53085d
Merge branch 'openSUSE:master' into auto-suggest-username
balsa-asanovic Feb 28, 2024
75b9781
logged changes
balsa-asanovic Feb 28, 2024
eb79c6f
Disable cspell in test file
dgdavid Feb 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 57 additions & 1 deletion web/src/components/users/FirstUser.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ import {
FormGroup,
TextInput,
Skeleton,
Menu,
MenuContent,
MenuList,
MenuItem
} from "@patternfly/react-core";

import { Table, Thead, Tr, Th, Tbody, Td } from '@patternfly/react-table';
Expand Down Expand Up @@ -97,6 +101,8 @@ export default function FirstUser() {
const [isFormOpen, setIsFormOpen] = useState(false);
const [isValidPassword, setIsValidPassword] = useState(true);
const [isSettingPassword, setIsSettingPassword] = useState(false);
const [showSuggestions, setShowSuggestions] = useState(false);
const [insideDropDown, setInsideDropDown] = useState(false);

useEffect(() => {
cancellablePromise(client.users.getUser()).then(userValues => {
Expand Down Expand Up @@ -166,6 +172,26 @@ export default function FirstUser() {
setFormValues({ ...formValues, [name]: value });
};

const suggestUsernames = (fullName) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll start immediately with some uncertainties 😄

When the user types two words for the full name, these current suggestions work well.

However, if the user's full name has more than two words (names), the choice is not so clear.
People having longer full names is a common thing in some parts of the world (like Spain) and with the current approach to suggesting the username, Ronaldo Luís Nazário de Lima would be presented with the following choices:

image

The issues are apparent, the first suggestion is pretty long, the second might be as well and only the third one might be okay.

Should we maybe take only the first two words (names)?
Or maybe some other approach entirely, feel free to make a suggestion.

Copy link
Contributor

@dgdavid dgdavid Feb 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I see your point, but I fear that guessing which are the right or expected parts is a bit difficult due to the fact on how different can be personal names around the world (long and old article).

Therefore, I wouldn't worry too much about them. They are just suggestions. Of course, the idea is to show up the more sensible ones, but we can't do so for every single user. I propose below as first. Some of them are based on feedback received in the issue, some are not:

  • Just the first word (the simplest approach)

    To make @kobliha happy

    • Lubos Kocman: lubos
    • Imobach González Sosa: imobach
    • Ronaldo Luís Nazário de Lima: ronaldo

  • First letter of first word plus second word

    It satisfies the original request from @lkocman

    • Lubos Kocman: lkocman
    • Imobach González Sosa: igonzalez
    • Ronaldo Luís Nazário de Lima: rluis

  • First letter of first word plus all other words

    It would suggest a username like the one @imobachgs have at SUSE

    • Lubos Kocman: lkocman
    • Imobach González Sosa: igonzalezsosa
    • Ronaldo Luís Nazário de Lima: rluisnazariodelima

  • First word plus first letter of the other words (maybe with a limit of just 2 more words?)

    • Lubos Kocman: lubosk
    • Imobach González Sosa: imobachgs
    • Ronaldo Luís Nazário de Lima: ronaldolndl (or ronaldoln if using a limit)

  • First letter of all words except the last one, which would be complete

    • Lubos Kocman: lkocman
    • Imobach González Sosa: igsosa
    • Ronaldo Luís Nazário de Lima: rlndlima

  • All words

    • Lubos Kocman: luboskocman
    • Imobach González Sosa: imobachgonzalezsosa
    • Ronaldo Luís Nazário de Lima: ronaldoluiznaariodelima

So, after removing duplicates, suggestions for these users will be,

Lubos Kocman Imobach González Sosa Ronaldo Luís Nazário de Lima
  • lubos
  • lkocman
  • lubosk
  • imobach
  • igonzalez
  • igonzalezsosa
  • imobachgs
  • igsosa
  • imobachgonzalezsosa
  • ronaldo
  • rluis
  • rluisnazariodelima
  • ronaldolndl (or ronaldoln)
  • rlndlima
  • ronaldoluiznaariodelima

I do not have strong preference and ideas from others are welcome. I think that we can release the feature with these few (or even only the three or four first ones) and wait for more suggestion requests.

BTW, as you can see in my examples above, I prefer not using the "special characters" in the suggested username (accents and friends). See https://stackoverflow.com/a/70288180 and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize

const cleanedName = fullName.replace(/[^\p{L}\p{N} ]/gu, "").toLowerCase();
Copy link
Contributor

@dgdavid dgdavid Feb 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

NP: At least for me, that line deserves an explanation / comment 🤯

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I'm not sure if it pays off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

NP: At least for me, that line deserves an explanation / comment 🤯

Haha, indeed, this was weird for me as well.
This has almost identical effect as the regular expression /[^a-zA-Z0-9 ]/g, which cleans the string from all characters except alphanumeric and blank spaces.
However, when I just used this initially, non English characters (like š, ć, č, ...) would also get removed.

So, after googling for a bit, I found that since ECMAScript 2018 Unicode property escapes are available for regular expressions in JavaScript. So, the '\p{L}' matches any character from any language and '\p{N}' matches any numeric character.

Also, I'm not sure if it pays off.

Of course, it is another thing should something like this be used at all. When I coded this, the idea was that users could enter some special characters in their full names, like ',', '.', ';', '-', '"', etc,... And I supposed that this isn't something that should be a part of the username.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks a lot for the explanation!

If we keep it in the code, definitely it deserves a comment above it.

However, I think that "johnq.diaz" is a valid username for "John Q. Díaz" where "johnqdíaz" is not. If I'm right, we can sanitize the name in another way or with other regular expression. Either way, a comment in the code is more than welcome for future developers.

Keep going!

const nameParts = cleanedName.split(/\s+/);
const suggestions = [];

nameParts.forEach((namePart, index) => {
if (index === 0) {
suggestions.push(namePart);
suggestions.push(namePart[0]);
nameParts.length > 1 && suggestions.push(namePart);
} else {
suggestions[0] += namePart;
suggestions[1] += namePart;
suggestions[2] += namePart[0];
}
});

return suggestions;
};
dgdavid marked this conversation as resolved.
Show resolved Hide resolved

const isUserDefined = user?.userName && user?.userName !== "";
const showErrors = () => ((errors || []).length > 0);

Expand Down Expand Up @@ -210,7 +236,14 @@ export default function FirstUser() {
/>
</FormGroup>

<FormGroup fieldId="userName" label={_("Username")} isRequired>
<FormGroup
className="first-username-wrapper"
fieldId="userName"
label={_("Username")}
isRequired
onFocus={() => setShowSuggestions(true)}
onBlur={() => !insideDropDown && setShowSuggestions(false)}
>
<TextInput
id="userName"
name="userName"
Expand All @@ -220,6 +253,29 @@ export default function FirstUser() {
isRequired
onChange={handleInputChange}
/>
{ showSuggestions && formValues.fullName && !formValues.userName &&
<Menu
aria-label={_("Username suggestion dropdown")}
className="first-username-dropdown"
onMouseEnter={() => setInsideDropDown(true)}
onMouseLeave={() => setInsideDropDown(false)}
dgdavid marked this conversation as resolved.
Show resolved Hide resolved
>
<MenuContent>
<MenuList>
{ suggestUsernames(formValues.fullName).map((suggestion, index) => (
<MenuItem
key={index}
itemId={index}
onClick={() => { setInsideDropDown(false); setFormValues({ ...formValues, userName: suggestion }) }}
>
{ /* TRANSLATORS: dropdown username suggestions */ }
{ _("Use suggested username ") }
dgdavid marked this conversation as resolved.
Show resolved Hide resolved
<strong>{suggestion}</strong>
dgdavid marked this conversation as resolved.
Show resolved Hide resolved
</MenuItem>
)) }
</MenuList>
</MenuContent>
</Menu> }
</FormGroup>

{ isEditing &&
Expand Down