Re: [Freeipa-devel] [PATCH] 730-732 webui: Login pages usability improvements
On 12.8.2014 22:58, Endi Sukma Dewata wrote: On 8/5/2014 6:36 AM, Petr Vobornik wrote: [PATCH] 730 webui: display expired session notification in a more visible area The notification is a primary information of the page. It should be more highlighted. https://fedorahosted.org/freeipa/ticket/4470 ACK. [PATCH] 731 webui: improved info msgs on login/token sync/reset pwd pages - add info icons to distinguish and classify the messages. - add info text for OTP fields - fix login instruction inaccuracy related to position of login button https://fedorahosted.org/freeipa/ticket/4470 Just one thing, instead of enter them in the fields nearby how about enter them in the corresponding fields? Otherwise it's ACKed. Changed, pushed using trivial/one-liner rule [PATCH] 732 webui: login screen - improved button switching - added cancel button to reset password view of login screen - re-implemented buttons hiding mechanism - switching between 'Reset Password' and 'Reset Password and Login' according to presence of value in OTP field https://fedorahosted.org/freeipa/ticket/4470 The code seems to be fine so it's ACKed, but see comments below: 1. It looks like the OTP token needs to be synchronized before it can be used for the first time. Is that true for all types of tokens (hardware/software, TOTP/HOTP)? If possible the synchronization should be part of the token creation process, so the admin can provide a token that can be used right away, so we may need an interface in the UI to sync the tokens. If the sync can only be done by users themselves, there should be a message on the login screen for first time OTP users to synchronize the token first. Synchronization right away won't hurt but it's not always required. TOTP works for me if the device has properly synchronized time. I haven't noticed any sync issue with HOTP. Synchronization right from the UI is covered by: https://fedorahosted.org/freeipa/ticket/4365 https://fedorahosted.org/freeipa/ticket/4366 2. Try logging in with an incorrect password/OTP. After you get a login error click Sync OTP Token. Once the sync is completed it will go back to the login page with a Token was synchronized message that disappears in a few seconds, but the old login error still appears which is confusing. Error messages in the UI should only reflect the last executed operation. I'll fix it in separate patch. -- Endi S. Dewata Pushed to: master: * a94fc09b5747ff5ffc632d95b330470ed78ee0f5 webui: display expired session notification in a more visible area * cba5247f99bca6eb8ed73b73f20cb9e9b3a45e91 webui: improved info msgs on login/token sync/reset pwd pages * 4832f2986d1a457acf3ff000433aa0732364c19c webui: login screen - improved button switching ipa-4-1: * 6f8dc9dba488caba7be2afc17b9e2b5191ffa585 webui: display expired session notification in a more visible area * 68647276ed58cb46c64884c2944cbd90979faf79 webui: improved info msgs on login/token sync/reset pwd pages * b37854051d6afd3f57ce28d059105797d13f0c22 webui: login screen - improved button switching -- Petr Vobornik ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 730-732 webui: Login pages usability improvements
On 8/5/2014 6:36 AM, Petr Vobornik wrote: [PATCH] 730 webui: display expired session notification in a more visible area The notification is a primary information of the page. It should be more highlighted. https://fedorahosted.org/freeipa/ticket/4470 ACK. [PATCH] 731 webui: improved info msgs on login/token sync/reset pwd pages - add info icons to distinguish and classify the messages. - add info text for OTP fields - fix login instruction inaccuracy related to position of login button https://fedorahosted.org/freeipa/ticket/4470 Just one thing, instead of enter them in the fields nearby how about enter them in the corresponding fields? Otherwise it's ACKed. [PATCH] 732 webui: login screen - improved button switching - added cancel button to reset password view of login screen - re-implemented buttons hiding mechanism - switching between 'Reset Password' and 'Reset Password and Login' according to presence of value in OTP field https://fedorahosted.org/freeipa/ticket/4470 The code seems to be fine so it's ACKed, but see comments below: 1. It looks like the OTP token needs to be synchronized before it can be used for the first time. Is that true for all types of tokens (hardware/software, TOTP/HOTP)? If possible the synchronization should be part of the token creation process, so the admin can provide a token that can be used right away, so we may need an interface in the UI to sync the tokens. If the sync can only be done by users themselves, there should be a message on the login screen for first time OTP users to synchronize the token first. 2. Try logging in with an incorrect password/OTP. After you get a login error click Sync OTP Token. Once the sync is completed it will go back to the login page with a Token was synchronized message that disappears in a few seconds, but the old login error still appears which is confusing. Error messages in the UI should only reflect the last executed operation. -- Endi S. Dewata ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 730-732 webui: Login pages usability improvements
[PATCH] 730 webui: display expired session notification in a more visible
area
The notification is a primary information of the page. It should be more
highlighted.
https://fedorahosted.org/freeipa/ticket/4470
[PATCH] 731 webui: improved info msgs on login/token sync/reset pwd pages
- add info icons to distinguish and classify the messages.
- add info text for OTP fields
- fix login instruction inaccuracy related to position of login button
https://fedorahosted.org/freeipa/ticket/4470
[PATCH] 732 webui: login screen - improved button switching
- added cancel button to reset password view of login screen
- re-implemented buttons hiding mechanism
- switching between 'Reset Password' and 'Reset Password and Login'
according to presence of value in OTP field
https://fedorahosted.org/freeipa/ticket/4470
--
Petr Vobornik
From 8c0afe1848a836c3aaa2be1a016e0ad3739b6009 Mon Sep 17 00:00:00 2001
From: Petr Vobornik pvobo...@redhat.com
Date: Fri, 25 Jul 2014 18:26:47 +0200
Subject: [PATCH] webui: login screen - improved button switching
- added cancel button to reset password view of login screen
- re-implemented buttons hiding mechanism
- switching between 'Reset Password' and 'Reset Password and Login' according to presence of value in OTP field
https://fedorahosted.org/freeipa/ticket/4470
---
install/ui/src/freeipa/widgets/LoginScreen.js | 68 +--
install/ui/src/freeipa/widgets/LoginScreenBase.js | 2 +
2 files changed, 54 insertions(+), 16 deletions(-)
diff --git a/install/ui/src/freeipa/widgets/LoginScreen.js b/install/ui/src/freeipa/widgets/LoginScreen.js
index be5cc7788f315990b6c17c8ddb2ff972513f412e..c6e14bfdb5e7d7bdec2bb5860996c1cc3b62520e 100644
--- a/install/ui/src/freeipa/widgets/LoginScreen.js
+++ b/install/ui/src/freeipa/widgets/LoginScreen.js
@@ -83,32 +83,69 @@ define(['dojo/_base/declare',
render_buttons: function(container) {
this.sync_btn_node = IPA.button({
+name: 'sync',
label: text.get('@i18n:login.sync_otp_token', Sync OTP Token),
button_class: 'btn btn-link',
click: lang.hitch(this, this.on_sync)
})[0];
construct.place(this.sync_btn_node, container);
+construct.place(document.createTextNode( ), container);
this.login_btn_node = IPA.button({
+name: 'login',
label: text.get('@i18n:login.login', Login),
'class': 'btn-primary btn-lg',
click: lang.hitch(this, this.on_confirm)
})[0];
construct.place(this.login_btn_node, container);
+construct.place(document.createTextNode( ), container);
+
+this.cancel_btn_node = IPA.button({
+name: 'cancel',
+label: text.get('@i18n:buttons.cancel', Cancel),
+'class': 'btn-default',
+click: lang.hitch(this, this.on_cancel)
+})[0];
+construct.place(this.cancel_btn_node, container);
+construct.place(document.createTextNode( ), container);
this.reset_btn_node = IPA.button({
+name: 'reset',
+label: text.get('@i18n:buttons.reset_password', Reset Password),
+'class': 'btn-primary btn-lg',
+click: lang.hitch(this, this.on_confirm)
+})[0];
+construct.place(this.reset_btn_node, container);
+construct.place(document.createTextNode( ), container);
+
+this.reset_and_login_btn_node = IPA.button({
+name: 'reset_and_login',
label: text.get('@i18n:buttons.reset_password_and_login', Reset Password and Login),
'class': 'btn-primary btn-lg',
click: lang.hitch(this, this.on_confirm)
})[0];
+construct.place(this.reset_and_login_btn_node, container);
+},
+
+set_visible_buttons: function(buttons) {
+if (!this.buttons_node) return;
+query('button', this.buttons_node).forEach(function(el) {
+if (buttons.indexOf(el.name) -1) {
+dom_style.set(el, 'display', '');
+} else {
+dom_style.set(el, 'display', 'none');
+}
+});
},
post_create_fields: function() {
var u_f = this.get_field('username');
var p_f = this.get_field('password');
+var otp_f = this.get_field('otp');
u_f.on('value-change', lang.hitch(this, this.on_form_change));
p_f.on('value-change', lang.hitch(this, this.on_form_change));
+otp_f.on('value-change', lang.hitch(this, this.on_otp_change));
this.on_form_change();
},
@@ -122,6 +159,15 @@ define(['dojo/_base/declare',
p_f.set_required(required);
},
+on_otp_change: function(event) {
+
