Details

    • Testing Instructions:
      Hide

      Test pre-requisites

      • 1 standard forum
      • Use your HTML inspector and screen reader to assert that the tests are correct

      Test steps

      1. Go to the standard forum and create a new topic
      2. Save the new topic and follow the 'Reply' link to add another post
      3. Make sure the page scrolls to the form because of the anchor in the URL
      4. Make sure there is a heading H2 (hidden) above the form
      5. Make sure it makes sense and is helpful using a screen header
      Show
      Test pre-requisites 1 standard forum Use your HTML inspector and screen reader to assert that the tests are correct Test steps Go to the standard forum and create a new topic Save the new topic and follow the 'Reply' link to add another post Make sure the page scrolls to the form because of the anchor in the URL Make sure there is a heading H2 (hidden) above the form Make sure it makes sense and is helpful using a screen header
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30909-master
    • Rank:
      33917

      Description

      Screen reader users may have a hard time finding the actual box to type their reply in after clicking the "Reply" link because it is so far down on the page.

      This problem should be considered in the context of redesigning the entire forum. One easy solution is to put a heading at the reply entry location.

        Issue Links

          Activity

          Hide
          Frédéric Massart added a comment -

          I am providing here an alternative solution. I'm afraid that setting a header looks too much like a hack where as using an anchor would do the trick. Please let me know what you think about that solution.

          Show
          Frédéric Massart added a comment - I am providing here an alternative solution. I'm afraid that setting a header looks too much like a hack where as using an anchor would do the trick. Please let me know what you think about that solution.
          Hide
          Ankit Agarwal added a comment -

          Hi Fred,
          I am not sure how helpful an hidden h2 is for screen readers, but under the given case this seems most feasible solution.
          Feel free to push for integration.

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Fred, I am not sure how helpful an hidden h2 is for screen readers, but under the given case this seems most feasible solution. Feel free to push for integration. Thanks
          Hide
          Sam Hemelryk added a comment -

          Hi Fred,

          After much deliberation over this seemingly simple issue I've decided for the time being that we should hold off on this change.

          I think the use of the anchor to automatically jump to the reply form is a good idea, although I think perhaps in master only we should give it a proper id to make the link more meaningful.

          Like Ankit the hidden heading strikes me as a bit of an odd ball. Having had a play with the post form and seeing why it is needed I really think that this approach is less than ideal.
          While accessibility is a important point I think it shouldn't be acquired at the cost of sensibility and functionality.

          The issue with the headings appears to be mostly because mforms heading element gets translated into a fieldset and legend rather than an actual heading and I gather that because this is being addressed screen readers don't cope with it too well.
          Assuming so then I think perhaps the best thing to do would be to address the design of that form, and either using a static/html element, or using some clever display style the form similar to what it is now but using a header.
          Of course a couple of things factor into whether this should be done or not, mainly that forum is due to be looked at and likely will be replaced.
          Anyway it's not really my decision to make (heh I'm certainly no accessibility expert) I'm sure that this can be talked about at the stable scrum and I'll let you guys decide.

          However from me this gets a -0.5. I think the use of the anchor is good, but could be improved for master. But I don't like the new hidden heading.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Fred, After much deliberation over this seemingly simple issue I've decided for the time being that we should hold off on this change. I think the use of the anchor to automatically jump to the reply form is a good idea, although I think perhaps in master only we should give it a proper id to make the link more meaningful. Like Ankit the hidden heading strikes me as a bit of an odd ball. Having had a play with the post form and seeing why it is needed I really think that this approach is less than ideal. While accessibility is a important point I think it shouldn't be acquired at the cost of sensibility and functionality. The issue with the headings appears to be mostly because mforms heading element gets translated into a fieldset and legend rather than an actual heading and I gather that because this is being addressed screen readers don't cope with it too well. Assuming so then I think perhaps the best thing to do would be to address the design of that form, and either using a static/html element, or using some clever display style the form similar to what it is now but using a header. Of course a couple of things factor into whether this should be done or not, mainly that forum is due to be looked at and likely will be replaced. Anyway it's not really my decision to make (heh I'm certainly no accessibility expert) I'm sure that this can be talked about at the stable scrum and I'll let you guys decide. However from me this gets a -0.5. I think the use of the anchor is good, but could be improved for master. But I don't like the new hidden heading. Cheers Sam
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Ankit Agarwal added a comment -

          Hi Sam,
          I agree that we should use a nice id for the form.
          About the header thing, I am not sure we can do a lot about it. The possible solution that I can think of are :-
          1- Redo form design to use headers instead of legends/fieldsets
          2- Use hidden header.
          3- Display both header and legend
          4- Do nothing about the header.

          my +1 for any 2 and 4

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Sam, I agree that we should use a nice id for the form. About the header thing, I am not sure we can do a lot about it. The possible solution that I can think of are :- 1- Redo form design to use headers instead of legends/fieldsets 2- Use hidden header. 3- Display both header and legend 4- Do nothing about the header. my +1 for any 2 and 4 Thanks
          Hide
          Frédéric Massart added a comment -

          Hi Sam,

          I have created a functionality to use custom IDs in forms and attached it to master. For 2.2 and 2.3 I am still using the ugly #mform1.

          Regarding the header, I agree that it is not really beautiful but we are already using H2 with accesshide at some other places. I have also investigated a way of using a heading in the forms but that looks really tricky as it would need a big refactoring. For now, it would not be possible to add (let's say) a addTitle method, or even a title element as they would be wrapped in a fieldset. Adding them outside a fieldset would either be a hack or changing formslib to be able to add something outside of a fieldset.

          Also, this specific form requires a heading but most of them don't, and when they have one it was added manually not using the mform.

          I am pushing this for peer review again.

          Cheers!

          Show
          Frédéric Massart added a comment - Hi Sam, I have created a functionality to use custom IDs in forms and attached it to master. For 2.2 and 2.3 I am still using the ugly #mform1. Regarding the header, I agree that it is not really beautiful but we are already using H2 with accesshide at some other places. I have also investigated a way of using a heading in the forms but that looks really tricky as it would need a big refactoring. For now, it would not be possible to add (let's say) a addTitle method, or even a title element as they would be wrapped in a fieldset. Adding them outside a fieldset would either be a hack or changing formslib to be able to add something outside of a fieldset. Also, this specific form requires a heading but most of them don't, and when they have one it was added manually not using the mform. I am pushing this for peer review again. Cheers!
          Hide
          Ankit Agarwal added a comment -

          Looks good Fred,
          This might need mentioning in dev docs, please make sure they are updated once this is integrated.

          Thanks

          Show
          Ankit Agarwal added a comment - Looks good Fred, This might need mentioning in dev docs, please make sure they are updated once this is integrated. Thanks
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Dan Poltawski added a comment -

          Hmmm.

          1. Could we just use an id tag on the h2 to jump to the right place? Would be less horrible
          2. Given the legacy heap which mforms is, it seems like putting a form within an element where we control the id would work for most cases, no? Rather than shoehorn the custom id, which worries me because everything in formslib worries me and we also have v.old JS depending on this kind of stuff.
          3. I'm not a fan of like the name $accessibleheading, does describe really what it does? Or is it only 'accessibleheading' at the point its put into a h2 with accesshide
          Show
          Dan Poltawski added a comment - Hmmm. Could we just use an id tag on the h2 to jump to the right place? Would be less horrible Given the legacy heap which mforms is, it seems like putting a form within an element where we control the id would work for most cases, no? Rather than shoehorn the custom id, which worries me because everything in formslib worries me and we also have v.old JS depending on this kind of stuff. I'm not a fan of like the name $accessibleheading, does describe really what it does? Or is it only 'accessibleheading' at the point its put into a h2 with accesshide
          Hide
          Dan Poltawski added a comment -

          Discussed with Fred.

          The problem with the h2 is that it won't work if its accesshiden and discussed my caution about the mforms thing.

          Fred is looking into the JS which me using this ID

          Show
          Dan Poltawski added a comment - Discussed with Fred. The problem with the h2 is that it won't work if its accesshiden and discussed my caution about the mforms thing. Fred is looking into the JS which me using this ID
          Hide
          Frédéric Massart added a comment -

          Hi Dan,

          as we discussed on the chat:

          • The ID on the H2 cannot be used to jump as it has the class accesshide and so position: absolute.
          • I understand that this could eventually create issues, but this is a master only change. There is a fallback on the current way of naming the forms. There does not seem to be any Javascript error on the forum page while using this ID. I haven't found hardcoded #mform1 in Javascript in the forum. So I think this can be integrated.
          • The variable has been renamed.

          Cheers!

          Show
          Frédéric Massart added a comment - Hi Dan, as we discussed on the chat: The ID on the H2 cannot be used to jump as it has the class accesshide and so position: absolute. I understand that this could eventually create issues, but this is a master only change. There is a fallback on the current way of naming the forms. There does not seem to be any Javascript error on the forum page while using this ID. I haven't found hardcoded #mform1 in Javascript in the forum. So I think this can be integrated. The variable has been renamed. Cheers!
          Hide
          Dan Poltawski added a comment -

          Thanks Fred, i've integrated this now (22, 23 and master)

          Show
          Dan Poltawski added a comment - Thanks Fred, i've integrated this now (22, 23 and master)
          Hide
          Greg Kraus added a comment -

          The h2 works there for screen reader users.

          Show
          Greg Kraus added a comment - The h2 works there for screen reader users.
          Hide
          Tim Barker added a comment -

          All good in 2.4

          Show
          Tim Barker added a comment - All good in 2.4
          Hide
          Tim Barker added a comment -

          all good in 2.3

          Show
          Tim Barker added a comment - all good in 2.3
          Hide
          Tim Barker added a comment -

          And finally it all works on 2.2 as well. Great work it's in!

          Show
          Tim Barker added a comment - And finally it all works on 2.2 as well. Great work it's in!
          Hide
          Dan Poltawski added a comment -

          Congratulations, you've done it!

          Thanks, this change is now in the latest weekly release!

          Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!

          Show
          Dan Poltawski added a comment - Congratulations, you've done it! Thanks, this change is now in the latest weekly release! Join the crowds of people tomorrow from 8am and download this Moodle release from your local apple store!
          Hide
          Juan Segarra Montesinos added a comment - - edited

          Hi guys. In 2.3, Reply buttons in forums are removed in MyMobile theme because of the anchor introduced at reply command (commit 6d3249780b401d7864ba61b57f55b4290f811e69).

          I've reported this in MDL-36305 (i've not found no suitable Component for reporting this :-/).

          Show
          Juan Segarra Montesinos added a comment - - edited Hi guys. In 2.3, Reply buttons in forums are removed in MyMobile theme because of the anchor introduced at reply command (commit 6d3249780b401d7864ba61b57f55b4290f811e69). I've reported this in MDL-36305 (i've not found no suitable Component for reporting this :-/).

            People

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

              Dates

              • Created:
                Updated:
                Resolved: