Hides the Sign in-button on 401 page #4101
Conversation
| @@ -237,7 +237,7 @@ define(['exports', 'jquery', 'oae.api.config', 'oae.api.i18n', 'oae.api.user', ' | |||
|
|
|||
| // Use the `auth.html` `authExternalButton` macro to create a form that performs this | |||
| // authentication | |||
| var $template = $('<div class="hide">${authExternalButton(strategy, opts)}</div>'); | |||
| var $template = $('<div><!--${authExternalButton(strategy, opts)}--></div>'); | |||
There was a problem hiding this comment.
My preference would be to do something along the following lines:
var $template = $('<div><!--' +
' <div class="hide">' +
' ${authExternalButton(strategy, opts)}' +
' </div>' +
'--></div>');
There was a problem hiding this comment.
We can then remove the addClass and it'll make it easier to make changes to this template further down the line
|
Finished review. Back to @sallakarppinen for follow-up |
| @@ -249,7 +249,7 @@ define(['exports', 'jquery', 'oae.api.config', 'oae.api.i18n', 'oae.api.user', ' | |||
| }); | |||
|
|
|||
| // Submit the form | |||
| $($.trim(form)).appendTo('body').submit(); | |||
| $($.trim(form)).addClass("hide").appendTo('body').submit(); | |||
There was a problem hiding this comment.
Can you confirm that triggering a submit on a hidden element isn't blocked by browsers?
|
Updated. .submit() was not working on Chrome/Chromium but submitting via .click() seems to work fine. |
| @@ -237,7 +237,11 @@ define(['exports', 'jquery', 'oae.api.config', 'oae.api.i18n', 'oae.api.user', ' | |||
|
|
|||
| // Use the `auth.html` `authExternalButton` macro to create a form that performs this | |||
| // authentication | |||
| var $template = $('<div class="hide">${authExternalButton(strategy, opts)}</div>'); | |||
| var $template = $('<div><!--' | |||
| + '<div class="hide">' | |||
There was a problem hiding this comment.
Maybe we can give this div an id so we can use that to limit the scope of the $('.btn-external-auth') selector below.
|
Left a comment. Back to @sallakarppinen |
|
Added id to the wrapper div. |
|
Looks good. Merged. |
Fixes issue #4092. As a sidenote, I tried to open this using
git pull-request -i 4092 -b oaeproject/3akai-ux:master -h sallakarppinen/3akai-ux:issue-4092 -m 'Hides the Sign in-button on 401 page'but it failed to link to the issue automatically (I've aliased hub as git).