Moodle

Allow disabledIf to hide form elements as well as disabling them

Details

  • Type: New Feature New Feature
  • Status: Closed Closed
  • Priority: Minor Minor
  • Resolution: Fixed
  • Affects Version/s: 1.9.5, 2.0
  • Fix Version/s: 1.9.6, 2.0
  • Component/s: Forms Library
  • Labels:
    None
  • Affected Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE
  • Fixed Branches:
    MOODLE_19_STABLE, MOODLE_20_STABLE

Description

In addition to the existing 'eq' and 'checked' operators in the disabledIf functionality, a 'hide' operator is added which tells the forms library to hide the form element as well as disabling it.
Patch applies to CVSHEAD and MOODLE_19_STABLE.

Issue Links

Activity

Hide
Jonathan Harker added a comment -

I'm still testing this with a client's Moodle. In particular, groups of radio buttons need testing.

TO TEST:
In your form definition, you will need an extra "hide" disabledIf line, in addition to the normal disabledIf condition(s). For example, an "other" text box that appears when the user picks "Other" from a select dropdown list:

$mform->disabledIf('othertext', 'occupationselect', 'neq', 'Other');
$mform->disabledIf('othertext', 'occupationselect', 'hide', '1');

Show
Jonathan Harker added a comment - I'm still testing this with a client's Moodle. In particular, groups of radio buttons need testing. TO TEST: In your form definition, you will need an extra "hide" disabledIf line, in addition to the normal disabledIf condition(s). For example, an "other" text box that appears when the user picks "Other" from a select dropdown list: $mform->disabledIf('othertext', 'occupationselect', 'neq', 'Other'); $mform->disabledIf('othertext', 'occupationselect', 'hide', '1');
Hide
Martin Dougiamas added a comment -

Jonathan, that looks very sensible to me. Sam when you get a minute can you try it out?

Show
Martin Dougiamas added a comment - Jonathan, that looks very sensible to me. Sam when you get a minute can you try it out?
Hide
Sam Hemelryk added a comment -

Hi Jonathan,
This patch is awesome thanks, the result is nice and its so simple.
I've attached a slightly modified patch moodle-formslib-disabledif-hide.20091001.01.diff that simply changes the were you were going parentNode.parentNode to a recursive method that keeps going until it gets to the disired div + class.
Let me know if you are happy with this.
Cheers
Sam

Show
Sam Hemelryk added a comment - Hi Jonathan, This patch is awesome thanks, the result is nice and its so simple. I've attached a slightly modified patch moodle-formslib-disabledif-hide.20091001.01.diff that simply changes the were you were going parentNode.parentNode to a recursive method that keeps going until it gets to the disired div + class. Let me know if you are happy with this. Cheers Sam
Hide
Sam Hemelryk added a comment -

Everything looks good Jonathan, feel free to commit this when want
Worth noting is that currently none of the disabledif stuff is working in Safari, please see the linked issue in regards to this.
Cheers
Sam

Show
Sam Hemelryk added a comment - Everything looks good Jonathan, feel free to commit this when want Worth noting is that currently none of the disabledif stuff is working in Safari, please see the linked issue in regards to this. Cheers Sam
Hide
Sam Hemelryk added a comment -

Thanks Jonathan for the patch, I've just submitted it now along with a fix for the bug that was preventing disabledif locking on Safari
Cheers
Sam

Show
Sam Hemelryk added a comment - Thanks Jonathan for the patch, I've just submitted it now along with a fix for the bug that was preventing disabledif locking on Safari Cheers Sam
Hide
Tim Hunt added a comment -

Actually, while this seems awesome, isn't it a big usability problem. It is, as far as I remember, very bad for screen-reader users if content appears and disappears. At least that is the advice I remember from a couple of years ago. That is why we originally went with disableIf, and not hideIf.

Can we get some advice from someone who is up-to-date on accessibility. I just added Nick Freear as a watcher. Hopefully he can comment.

Show
Tim Hunt added a comment - Actually, while this seems awesome, isn't it a big usability problem. It is, as far as I remember, very bad for screen-reader users if content appears and disappears. At least that is the advice I remember from a couple of years ago. That is why we originally went with disableIf, and not hideIf. Can we get some advice from someone who is up-to-date on accessibility. I just added Nick Freear as a watcher. Hopefully he can comment.
Hide
Sam Hemelryk added a comment -

Hi Tim, yes indeed I hadn't thought about screen readers. I'm keen to see what Nick (or any anyone else) has to say on the matter. Perhaps we need adjust for this and look for a solution that will work for screen readers as well.

Show
Sam Hemelryk added a comment - Hi Tim, yes indeed I hadn't thought about screen readers. I'm keen to see what Nick (or any anyone else) has to say on the matter. Perhaps we need adjust for this and look for a solution that will work for screen readers as well.
Hide
Tim Hunt added a comment -

Yes, if you look at what solutions the WAI-ARIA draft spec is planning to add to HTML, you can deduce the kinds of problems users of assistive technology have with currently ajax-y web pages: http://www.w3.org/TR/wai-aria-practices/#relations_controls.

We should keep an eye on WAI-ARIA, I think.

Show
Tim Hunt added a comment - Yes, if you look at what solutions the WAI-ARIA draft spec is planning to add to HTML, you can deduce the kinds of problems users of assistive technology have with currently ajax-y web pages: http://www.w3.org/TR/wai-aria-practices/#relations_controls. We should keep an eye on WAI-ARIA, I think.
Hide
Petr Škoda (skodak) added a comment -

The hiding was not implemented intentionally in our formslib library for accessibility reasons.

Show
Petr Škoda (skodak) added a comment - The hiding was not implemented intentionally in our formslib library for accessibility reasons.
Hide
John Shaver added a comment -

Hide does not work for a group of radiobuttons... Have added:

case 'hide':
// hide as well as disable
lock = get_form_element_value(form, dependon) != value;
hide = true; break;

Lines 199 - 202

In lib/javascript-static.js.... This is probley not the best way but it works..

Show
John Shaver added a comment - Hide does not work for a group of radiobuttons... Have added: case 'hide': // hide as well as disable lock = get_form_element_value(form, dependon) != value; hide = true; break; Lines 199 - 202 In lib/javascript-static.js.... This is probley not the best way but it works..
Hide
John Shaver added a comment -

Also note that 'hide' does not work on form headers...

Show
John Shaver added a comment - Also note that 'hide' does not work on form headers...

People

Dates

  • Created:
    Updated:
    Resolved: