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
    • Rank:
      6302

      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

      1. 20110112-Fix_Default_Button_For_User_Search.patch
        0.8 kB
        Jason Ilicic
      2. MDL-24200.patch
        0.5 kB
        Hubert Chathi

        Issue Links

          Activity

          Helen Foster created issue -
          Martin Dougiamas made changes -
          Field Original Value New Value
          Workflow jira [ 38664 ] MDL Workflow [ 41461 ]
          Hide
          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
          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'.
          Jason Ilicic made changes -
          Show
          Jason Ilicic added a comment - Added patch to GitHub - https://github.com/jasonilicic/moodle/commit/6633055c10938365848a010dba81e06107071cb4
          Hide
          Helen Foster added a comment -

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

          Show
          Helen Foster added a comment - Jason, thanks for your comments and patch. Hoping one of our HQ devs can review it soon.
          Helen Foster made changes -
          Labels triaged
          Fix Version/s STABLE backlog [ 10463 ]
          Component/s Usability [ 10309 ]
          Helen Foster made changes -
          Labels triaged patch triaged
          Helen Foster made changes -
          Link This issue is duplicated by MDL-26446 [ MDL-26446 ]
          Martin Dougiamas made changes -
          Workflow MDL Workflow [ 41461 ] MDL Full Workflow [ 74980 ]
          Anthony Borrow made changes -
          Link This issue is duplicated by MDL-25538 [ MDL-25538 ]
          Hide
          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
          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.
          Hubert Chathi made changes -
          Attachment MDL-24200.patch [ 25324 ]
          Hide
          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
          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!
          Jason Fowler made changes -
          Assignee moodle.com [ moodle.com ] Jason Fowler [ phalacee ]
          Hide
          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
          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.
          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 made changes -
          Peer reviewer 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
          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
          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.
          Rajesh Taneja made changes -
          Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
          Hide
          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
          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
          Jason Fowler made changes -
          Link This issue has a non-specific relationship to MDL-30110 [ MDL-30110 ]
          Jason Fowler made changes -
          Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
          Aparup Banerjee made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Integrator nebgor
          Aparup Banerjee made changes -
          Status Integration review in progress [ 10004 ] Waiting for integration review [ 10010 ]
          Aparup Banerjee made changes -
          Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
          Currently in integration Yes [ 10041 ]
          Hide
          Aparup Banerjee added a comment -

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

          Show
          Aparup Banerjee added a comment - Hi Jason, failing this as discussed. js error: this._advButtons.set is not a function
          Aparup Banerjee made changes -
          Status Integration review in progress [ 10004 ] Reopened [ 4 ]
          Jason Fowler made changes -
          Status Reopened [ 4 ] Development in progress [ 3 ]
          Jason Fowler made changes -
          Status Development in progress [ 3 ] Peer review in progress [ 10013 ]
          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
          Rajesh Taneja added a comment -

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

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

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

          Show
          Aparup Banerjee added a comment - Thanks for looking at this guys, its been integrated and is up for testing.
          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 Agarwal made changes -
          Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
          Tester ankit_frenz
          Hide
          Ankit Agarwal added a comment -

          works perfectly!
          Test passed!
          Thanks!

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

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

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Yes, you got this finally upstream, just in time for Moodle 2.2beta. Congrats and thanks! Ciao
          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: