Modal form from masthead: Usercode Password Instructions Reset Register Login opac-auth (click Login from home page): Usercode Password Login Reset Instructions Register opac-main: Usercode Password Login Instructions Reset Register I would suggest to follow the pattern from the home page. After entering usercode and password, next step is normally click Login. Below that offer Instructions, Reset and Register. Do the same on modal and opac-auth.
Created attachment 127539 [details] [review] Bug 29458: Reorder login elements Make masthead.inc modal and opac-auth follow the order of the login elements on the homepage. Test plan: Enable prefs for reset password and self registration. Fill additional contents for login instructions. Logout on OPAC. Click Login from your account on top bar. Click Login button on the home page (right side, in the middle). Verify that these two forms correspond with the home page login. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 127828 [details] [review] Bug 29458: Reorder login elements Make masthead.inc modal and opac-auth follow the order of the login elements on the homepage. Test plan: Enable prefs for reset password and self registration. Fill additional contents for login instructions. Logout on OPAC. Click Login from your account on top bar. Click Login button on the home page (right side, in the middle). Verify that these two forms correspond with the home page login. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Looks way better :D In opac-auth, with should also fix : You may register here. instead of Don't have an account? Register here. Maybe in another bug.
(In reply to Fridolin Somers from comment #3) > Looks way better :D > > In opac-auth, with should also fix : > You may register here. > instead of > Don't have an account? Register here. > > Maybe in another bug. Please open another bug. Can we get this one further please ?
I opened Bug 29730
Please rebase. It like it but doesn't apply.
Created attachment 129240 [details] [review] Bug 29458: Reorder login elements Make masthead.inc modal and opac-auth follow the order of the login elements on the homepage. Test plan: Enable prefs for reset password and self registration. Fill additional contents for login instructions. Logout on OPAC. Click Login from your account on top bar. Click Login button on the home page (right side, in the middle). Verify that these two forms correspond with the home page login. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
(In reply to Tomás Cohen Arazi from comment #6) > Please rebase. It like it but doesn't apply. Easy fix :)
+ <div class="">[%# FIXME Add right class, remove br %] + <br> Easy fixme, should be implemented here.
Created attachment 129606 [details] [review] Bug 29458: (follow-up) Remove FIXME Actually, just leaving the br tag is fine too. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Owen, could you have a look at the second patch? Would you suggest to add a class and additional css or so instead?
Created attachment 129612 [details] [review] Bug 29458: (follow-up) Remove FIXME With some help of Owen Leonard: <oleonard> Regarding the <br/>, you can wrap the submit button in <fieldset class="action"></fieldset> instead. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
(In reply to Jonathan Druart from comment #9) > + <div class="">[%# FIXME Add right class, > remove br %] > + <br> > > Easy fixme, should be implemented here. Fixed now with help from Owen.
Created attachment 129617 [details] [review] Bug 29458: Reorder login elements Make masthead.inc modal and opac-auth follow the order of the login elements on the homepage. Test plan: Enable prefs for reset password and self registration. Fill additional contents for login instructions. Logout on OPAC. Click Login from your account on top bar. Click Login button on the home page (right side, in the middle). Verify that these two forms correspond with the home page login. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com> Bug 29458: (follow-up) Remove FIXME With some help of Owen Leonard: <oleonard> Regarding the <br/>, you can wrap the submit button in <fieldset class="action"></fieldset> instead. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Selenium authentication tests are failing because of this change: Error while executing command: element not interactable: Element <input class="btn btn-primary" type="submit"> could not be scrolled into view at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356.
(In reply to Jonathan Druart from comment #16) > Selenium authentication tests are failing because of this change: > Error while executing command: element not interactable: Element <input > class="btn btn-primary" type="submit"> could not be scrolled into view at > /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. > at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356. Which test specifically ?
(In reply to Marcel de Rooy from comment #17) > (In reply to Jonathan Druart from comment #16) > > Selenium authentication tests are failing because of this change: > > Error while executing command: element not interactable: Element <input > > class="btn btn-primary" type="submit"> could not be scrolled into view at > > /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. > > at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356. > > Which test specifically ? I have spent some time on setting up Selenium, and all its prerequisites within a Docker container. But I dont really have time for getting that working now. This sounds like a simple change required, but I cant provide or test it.
Created attachment 130016 [details] [review] Bug 29458: Fix selenium test The authentication.t selenium tests (and a couple of others) were failing with: Error while executing command: element not interactable: Element <input class="btn btn-primary" type="submit"> could not be scrolled into view at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. at /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356. We changed the other of the form, and t::lib::Selenium::submit_form was not getting the correct (first) form. The one from the auth modal was retrieved and submit button was clicked. Selenium raised an error as it is not displayed. The ->is_displayed selenium method does not work, as per the doc """ Note: This does *not* tell you an element's 'visibility' property; as it still takes up space in the DOM and is therefore considered 'displayed'. """ https://metacpan.org/pod/Selenium::Remote::WebElement#is_displayed "The internet" is saying we should be able to use the following in our xpath expression: not(ancestor::div[contains(@style,'display:none')] but it actually only works if the display:none rule is defined on the node (not from .css). Which does not work for us. The only solution I found is to check for the size of the element, which is (0,0) if not effectively displayed.
I am getting another selenium failure locally, with and without the patch. Not sure it's related to this, Jenkins has it passing. # at t/db_dependent/selenium/update_child_to_adult.t line 148. Cannot wait more for element '//input[@type="submit"]' to be visible at /kohadevbox/koha/t/lib/Selenium.pm line 189.
(In reply to Jonathan Druart from comment #19) > Created attachment 130016 [details] [review] [review] > Bug 29458: Fix selenium test > > The authentication.t selenium tests (and a couple of others) were > failing with: > Error while executing command: element not interactable: Element <input > class="btn btn-primary" type="submit"> could not be scrolled into view at > /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 411. at > /usr/local/share/perl/5.32.1/Selenium/Remote/Driver.pm line 356. > > We changed the other of the form, and t::lib::Selenium::submit_form was > not getting the correct (first) form. The one from the auth modal was > retrieved and submit button was clicked. Selenium raised an error as it > is not displayed. > The ->is_displayed selenium method does not work, as per the doc > """ > Note: This does *not* tell you an element's 'visibility' property; as it > still takes up space in the DOM and is therefore considered 'displayed'. > """ > https://metacpan.org/pod/Selenium::Remote::WebElement#is_displayed > > "The internet" is saying we should be able to use the following in our > xpath expression: not(ancestor::div[contains(@style,'display:none')] > but it actually only works if the display:none rule is defined on the > node (not from .css). Which does not work for us. > > The only solution I found is to check for the size of the element, which > is (0,0) if not effectively displayed. Thanks. Too bad that we need such workarounds.
Change status to ?
(In reply to Marcel de Rooy from comment #22) > Change status to ? No need to, there is the additional_work_needed keyword.
> Bug 29458: Fix selenium test Pushed to master
Not backported to 21.05.x