Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-24200

Browse list of users focusses on 'Show advanced' button instead of 'Add filter'

    Details

    • Testing Instructions:
      Hide
      1. go to admin/user.php within your moodle instance (with javascript enabled)
      2. use firebug or equiv to check that the "show advanced" button is now a button and not a submit
      3. type text into the filter input text box
      4. hit enter on the keyboard

      The results should be filtered

      Show
      go to admin/user.php within your moodle instance (with javascript enabled) use firebug or equiv to check that the "show advanced" button is now a button and not a submit type text into the filter input text box hit enter on the keyboard The results should be filtered
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-24200-master

      Description

      Steps to recreate:

      1. In admin/user.php enter a name
      2. Click the enter/return button on your keyboard

      Expected result: Search results are returned (i.e. behaviour as in 1.9)

      Actual result: Focus moves to the 'Show advanced' button

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              tsala Helen Foster created issue -
              dougiamas Martin Dougiamas made changes -
              Field Original Value New Value
              Workflow jira [ 38664 ] MDL Workflow [ 41461 ]
              Hide
              jaseeey Jason Ilicic added a comment -

              Issue occurs because there are two submit buttons within the form, where HTML will automatically force the default button to be the first of type 'submit' in the form. There is no way to choose a default button in HTML as this has not yet been implemented, but a way to work around it is by setting the "Show/hide advanced" button to type 'button', so "Add filter" is the first in order of type 'submit'.

              Show
              jaseeey Jason Ilicic added a comment - Issue occurs because there are two submit buttons within the form, where HTML will automatically force the default button to be the first of type 'submit' in the form. There is no way to choose a default button in HTML as this has not yet been implemented, but a way to work around it is by setting the "Show/hide advanced" button to type 'button', so "Add filter" is the first in order of type 'submit'.
              jaseeey Jason Ilicic made changes -
              Show
              jaseeey Jason Ilicic added a comment - Added patch to GitHub - https://github.com/jasonilicic/moodle/commit/6633055c10938365848a010dba81e06107071cb4
              Hide
              tsala Helen Foster added a comment -

              Jason, thanks for your comments and patch. Hoping one of our HQ devs can review it soon.

              Show
              tsala Helen Foster added a comment - Jason, thanks for your comments and patch. Hoping one of our HQ devs can review it soon.
              tsala Helen Foster made changes -
              Labels triaged
              Fix Version/s STABLE backlog [ 10463 ]
              Component/s Usability [ 10309 ]
              tsala Helen Foster made changes -
              Labels triaged patch triaged
              tsala Helen Foster made changes -
              Link This issue is duplicated by MDL-26446 [ MDL-26446 ]
              dougiamas Martin Dougiamas made changes -
              Workflow MDL Workflow [ 41461 ] MDL Full Workflow [ 74980 ]
              aborrow Anthony Borrow made changes -
              Link This issue is duplicated by MDL-25538 [ MDL-25538 ]
              Hide
              hchathi Hubert Chathi added a comment - - edited

              We can't just change the type from submit to button in forsmlib – otherwise it will break the functionality for people who are not using JavaScript. Attached is a patch (MDL-24200.patch) that will change the type within the JavaScript itself.

              Show
              hchathi Hubert Chathi added a comment - - edited We can't just change the type from submit to button in forsmlib – otherwise it will break the functionality for people who are not using JavaScript. Attached is a patch ( MDL-24200 .patch) that will change the type within the JavaScript itself.
              hchathi Hubert Chathi made changes -
              Attachment MDL-24200.patch [ 25324 ]
              Hide
              tsala Helen Foster added a comment -

              Thanks Hubert. I run into this bug at least once a day so would love for it to be fixed!

              Show
              tsala Helen Foster added a comment - Thanks Hubert. I run into this bug at least once a day so would love for it to be fixed!
              phalacee Jason Fowler made changes -
              Assignee moodle.com [ moodle.com ] Jason Fowler [ phalacee ]
              Hide
              phalacee Jason Fowler added a comment -

              Well, I've had a play with this, and Hubert's patch works while Javascript is on, but more work needs to be done to fix it over all. Basically, the way the forms are expanded through-out Moodle would need to be looked into to fix this issue.

              Show
              phalacee Jason Fowler added a comment - Well, I've had a play with this, and Hubert's patch works while Javascript is on, but more work needs to be done to fix it over all. Basically, the way the forms are expanded through-out Moodle would need to be looked into to fix this issue.
              phalacee Jason Fowler made changes -
              Status Open [ 1 ] Waiting for peer review [ 10012 ]
              Pull Master Diff URL https://github.com/phalacee/moodle/compare/master...wip-MDL-24200-master
              Pull Master Branch wip-MDL-24200-master
              Pull from Repository git://github.com/phalacee/moodle.git
              moodle.com moodle.com made changes -
              Peer reviewer rajeshtaneja
              rajeshtaneja Rajesh Taneja made changes -
              Original Estimate 0 minutes [ 0 ]
              Remaining Estimate 0 minutes [ 0 ]
              Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
              Hide
              rajeshtaneja Rajesh Taneja added a comment - - edited

              This is a partial solution, which will work for browsers with JS.
              After discussion with you, I think it is fine to have this partial fix and have a new bug to write "show advance" functionality to handle this situation in non-js supported browsers.

              Also, please backport this as well.

              Show
              rajeshtaneja Rajesh Taneja added a comment - - edited This is a partial solution, which will work for browsers with JS. After discussion with you, I think it is fine to have this partial fix and have a new bug to write "show advance" functionality to handle this situation in non-js supported browsers. Also, please backport this as well.
              rajeshtaneja Rajesh Taneja made changes -
              Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
              Hide
              phalacee Jason Fowler added a comment -

              my ideal way of handling it would be to render the whole form, and then use Javascript to hide parts of the form for the advanced view, and add a button that when clicked uses javascript to reveal the hidden parts form. That way if javascript is off, those parts of the form are there already, and the "Show Advanced" button is not needed

              Show
              phalacee Jason Fowler added a comment - my ideal way of handling it would be to render the whole form, and then use Javascript to hide parts of the form for the advanced view, and add a button that when clicked uses javascript to reveal the hidden parts form. That way if javascript is off, those parts of the form are there already, and the "Show Advanced" button is not needed
              phalacee Jason Fowler made changes -
              Link This issue has a non-specific relationship to MDL-30110 [ MDL-30110 ]
              phalacee Jason Fowler made changes -
              Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
              nebgor Aparup Banerjee made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Integrator nebgor
              nebgor Aparup Banerjee made changes -
              Status Integration review in progress [ 10004 ] Waiting for integration review [ 10010 ]
              nebgor Aparup Banerjee made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Currently in integration Yes [ 10041 ]
              Hide
              nebgor Aparup Banerjee added a comment -

              Hi Jason, failing this as discussed.
              js error: this._advButtons.set is not a function

              Show
              nebgor Aparup Banerjee added a comment - Hi Jason, failing this as discussed. js error: this._advButtons.set is not a function
              nebgor Aparup Banerjee made changes -
              Status Integration review in progress [ 10004 ] Reopened [ 4 ]
              phalacee Jason Fowler made changes -
              Status Reopened [ 4 ] Development in progress [ 3 ]
              phalacee Jason Fowler made changes -
              Status Development in progress [ 3 ] Peer review in progress [ 10013 ]
              phalacee Jason Fowler made changes -
              Testing Instructions # go to admin/user.php within your moodle instance (with javascript enabled)
              # use firebug or equiv to check that the "show advanced" button is now a button and not a submit
              # type text into the filter input text box
              # hit enter on the keyboard

              The results should be filtered
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Patch works fine now...
              Patch provided by Hubert is spot-on

              Show
              rajeshtaneja Rajesh Taneja added a comment - Patch works fine now... Patch provided by Hubert is spot-on
              rajeshtaneja Rajesh Taneja made changes -
              Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
              rajeshtaneja Rajesh Taneja made changes -
              Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
              nebgor Aparup Banerjee made changes -
              Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
              Hide
              nebgor Aparup Banerjee added a comment -

              Thanks for looking at this guys, its been integrated and is up for testing.

              Show
              nebgor Aparup Banerjee added a comment - Thanks for looking at this guys, its been integrated and is up for testing.
              nebgor Aparup Banerjee made changes -
              Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
              Affects Version/s 2.1.2 [ 10851 ]
              Affects Version/s 2.0.5 [ 10950 ]
              Affects Version/s 2.2 [ 10656 ]
              Affects Version/s 2.0 [ 10122 ]
              Fix Version/s 2.0.6 [ 11250 ]
              Fix Version/s 2.1.3 [ 11251 ]
              Fix Version/s STABLE backlog [ 10463 ]
              ankit_frenz Ankit Agarwal made changes -
              Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
              Tester ankit_frenz
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              works perfectly!
              Test passed!
              Thanks!

              Show
              ankit_frenz Ankit Agarwal added a comment - works perfectly! Test passed! Thanks!
              ankit_frenz Ankit Agarwal made changes -
              Status Testing in progress [ 10011 ] Tested [ 10006 ]
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks!

              Ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao
              stronk7 Eloy Lafuente (stronk7) made changes -
              Status Tested [ 10006 ] Closed [ 6 ]
              Resolution Fixed [ 1 ]
              Currently in integration Yes [ 10041 ]
              Integration date 15/Nov/11

                People

                • Votes:
                  6 Vote for this issue
                  Watchers:
                  6 Start watching this issue

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    28/Nov/11