Account manager should remain unambiguous when currently active user logs out. #139

2020-06-17T12:30:23+02:00

yrammos commented

2020-06-17 12:30:23 +02:00

(Migrated from github.com)

Clicking on the Log Out button of the currently active user renders the radio control ambiguous, giving the false impression that no user is active.

This PR addresses the issue by executing the account change as soon as the radio option is changed. The "Select" button is thus no longer necessary and is replaced with a "Back" button.

If the currently active user is logged out, the first user on the list is automatically selected and the radio remains unambiguous.

In any case, a notification is used to confirm that the active account was indeed switched (or otherwise display an error).

Clicking on the Log Out button of the currently active user renders the radio control ambiguous, giving the false impression that no user is active. This PR addresses the issue by executing the account change as soon as the radio option is changed. The "Select" button is thus no longer necessary and is replaced with a "Back" button. If the currently active user is logged out, the first user on the list is automatically selected and the radio remains unambiguous. In any case, a notification is used to confirm that the active account was indeed switched (or otherwise display an error).

babelouest (Migrated from github.com) requested changes

2020-06-17 14:18:32 +02:00

babelouest (Migrated from github.com) left a comment

Nice job @yrammos , with the few requested changes, it will be good!

webapp-src/locales/fr/translations.json

Better use "Retour" in French

Better use `"Retour"` in French

webapp-src/src/Login/SelectAccount.js

					
				@ -28,18 +28,20 @@ class SelectAccount extends Component {

				    });

				  }

I like the new behavior, more simple and intuitive, but since the Select button has been replaced by a Back button, this handle should be renamed handleBack.

I like the new behavior, more simple and intuitive, but since the `Select` button has been replaced by a `Back` button, this handle should be renamed `handleBack`.

You don't need to use backquotes to surround the message, instead, add a new i18n entry:

messageDispatcher.sendMessage('Notification', {type: "success", message: i18next.t("login.select-login", {username: this.state.currentUser.username})});

And the new i18n entry in the json locale files should be like:

{
  "login": {
   // insert this in the "login block"
    "select-login": "User {{username}} selected"
  }
}

You don't need to use backquotes to surround the message, instead, add a new i18n entry: ```javascript messageDispatcher.sendMessage('Notification', {type: "success", message: i18next.t("login.select-login", {username: this.state.currentUser.username})}); ``` And the new i18n entry in the json locale files should be like: ```javascript { "login": { // insert this in the "login block" "select-login": "User {{username}} selected" } } ```

yrammos commented

2020-06-17 15:01:17 +02:00

(Migrated from github.com)

Thank you for the review, @babelouest. Done.

babelouest commented

2020-06-17 15:32:11 +02:00

(Migrated from github.com)

@yrammos , I merged your PR in my master. I took the liberty afterwards to make another change, there was a handleToggleGrantScope call in the render() which was not necessary, I also removed the checked attribute in the hidden checkbox, it's was worthless.

Thanks!

@yrammos , I merged your PR in my master. I took the liberty afterwards to make another change, there was a `handleToggleGrantScope` call in the `render()` which was not necessary, I also removed the `checked` attribute in the hidden checkbox, it's was worthless. Thanks!

🎉 1

Sign in to join this conversation.

No reviewers

babelouest