Details

    • Testing Instructions:
      Hide
      1. create a lesson within the course
      2. On lesson setting, enable progress bar
      3. add couple of questions
      4. as student, attempt the lesson and notice the progress bar on the bottom page

      Note: Make sure the progress bar is display properly.

      Show
      create a lesson within the course On lesson setting, enable progress bar add couple of questions as student, attempt the lesson and notice the progress bar on the bottom page Note: Make sure the progress bar is display properly.
    • Affected Branches:
      MOODLE_21_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      33918

      Description

      The progress bar is not read by screen reader users.

        Issue Links

          Activity

          Hide
          Rossiani Wijaya added a comment -

          Hi Glenn,

          I'm assigning you as peer-review. Feel free to re-assign this issue to other.

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Hi Glenn, I'm assigning you as peer-review. Feel free to re-assign this issue to other. Thanks Rosie
          Hide
          Glenn Ansley added a comment -

          Hi Rossiani,
          Looks good to me.

          Show
          Glenn Ansley added a comment - Hi Rossiani, Looks good to me.
          Hide
          Rossiani Wijaya added a comment -

          Thanks Glenn for reviewing.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Glenn for reviewing. Submitting for integration review.
          Hide
          Sam Hemelryk added a comment -

          Hi Rosie,

          I've sent this back just so that a couple of minor things can be tidied up:

          1. The new string incompleted, I don't really think that word is the best fit for what is going on. Could you please discuss it with Helen and see what she thinks.
          2. The commit message should have AMOS syntax to copy completed to the new completed string to save translators time.

          While looking at this I can't help but feel that a better approach to structuring the progress bar table to contain access hidden information could be used.
          What about simply constructing a string, something like "You have completed 33% of the lesson" implanting that in the page before the progress bar and then hiding it with accesshidden. Of course perhaps you may even want to show that to everyone.
          Glenn, Rosie what are your thoughts?

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Rosie, I've sent this back just so that a couple of minor things can be tidied up: The new string incompleted, I don't really think that word is the best fit for what is going on. Could you please discuss it with Helen and see what she thinks. The commit message should have AMOS syntax to copy completed to the new completed string to save translators time. While looking at this I can't help but feel that a better approach to structuring the progress bar table to contain access hidden information could be used. What about simply constructing a string, something like "You have completed 33% of the lesson" implanting that in the page before the progress bar and then hiding it with accesshidden. Of course perhaps you may even want to show that to everyone. Glenn, Rosie what are your thoughts? Cheers Sam
          Hide
          Rossiani Wijaya added a comment -

          Hi Sam,

          I like the new string suggestion, it describes the progress bar clearly for accessibility and others (assuming that we are not hiding it). However the table for the graph still required some accessibility definition for screen reader. I looked at this site for table's accessibility compliance (http://accessibility.psu.edu/tables).

          I will try to get in touch with Helen before the end of today.

          Show
          Rossiani Wijaya added a comment - Hi Sam, I like the new string suggestion, it describes the progress bar clearly for accessibility and others (assuming that we are not hiding it). However the table for the graph still required some accessibility definition for screen reader. I looked at this site for table's accessibility compliance ( http://accessibility.psu.edu/tables ). I will try to get in touch with Helen before the end of today.
          Hide
          Helen Foster added a comment -

          Hi Rosie and Sam,

          "You have completed 33% of the lesson" sounds good to me too!

          PS I tried to take a look at the progress bar (as I've never seen one before) by following the testing instructions above, however when I logged in as a student and attempted the lesson, I couldn't see a progress bar anywhere! Please can you advise where I should look for it.

          Show
          Helen Foster added a comment - Hi Rosie and Sam, "You have completed 33% of the lesson" sounds good to me too! PS I tried to take a look at the progress bar (as I've never seen one before) by following the testing instructions above, however when I logged in as a student and attempted the lesson, I couldn't see a progress bar anywhere! Please can you advise where I should look for it.
          Hide
          Rossiani Wijaya added a comment -

          Hi Helen,

          Try adding couple of question to the lesson and attempt the lesson as student.

          The progress bar should be available on the bottom of the page.

          Let me know if it works.

          Thanks
          Rosie

          Show
          Rossiani Wijaya added a comment - Hi Helen, Try adding couple of question to the lesson and attempt the lesson as student. The progress bar should be available on the bottom of the page. Let me know if it works. Thanks Rosie
          Hide
          Rossiani Wijaya added a comment -

          Hi Sam,

          I make changes to the patch. Instead of using table, it will use div with some styling. I think this should be simplified the accessibility compliance.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Hi Sam, I make changes to the patch. Instead of using table, it will use div with some styling. I think this should be simplified the accessibility compliance. Submitting for integration review.
          Hide
          Sam Hemelryk added a comment -

          Thanks Rosie this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks Rosie this has been integrated now
          Hide
          Rajesh Taneja added a comment -

          Thanks Rossie,

          Looks much better now
          Few things, I noticed:

          1. Old progress bar used to have a empty div (progress_bar_token) which is black rectangle after green, do we plan to support that?
          2. progress_bar_token is not used after applying this patch, do we need to remove this from css ?

          Rossie, Apu and I discussed this and we feel, this should be passed and we can open a new issue to clean mod/lesson/styles.css

          In addition to this, as this is a UI change, is it fine to have this on Stable branches? Apu and Rossie thinks it is fine.

          Also, I feel thinking if we can have a outer boundary for progress bar to show relative progress, ie total progress bar limit (probably another improvement).

          Show
          Rajesh Taneja added a comment - Thanks Rossie, Looks much better now Few things, I noticed: Old progress bar used to have a empty div (progress_bar_token) which is black rectangle after green, do we plan to support that? progress_bar_token is not used after applying this patch, do we need to remove this from css ? Rossie, Apu and I discussed this and we feel, this should be passed and we can open a new issue to clean mod/lesson/styles.css In addition to this, as this is a UI change, is it fine to have this on Stable branches? Apu and Rossie thinks it is fine. Also, I feel thinking if we can have a outer boundary for progress bar to show relative progress, ie total progress bar limit (probably another improvement).
          Hide
          Aparup Banerjee added a comment -

          Hi, just reading this now:

          1) no idea what that black rectangle is actually. first came in during a cleaning of lesson @ MDL-210060 (Sam's good old *fun days) , Sam's your man here Raj.

          2) i think that any extra/redundant css might be ok to clean up in master - but i'm not sure about stability issues for themes , again Sam's the man. oh, He's also the integrator here

          cheers!

          Show
          Aparup Banerjee added a comment - Hi, just reading this now: 1) no idea what that black rectangle is actually. first came in during a cleaning of lesson @ MDL-210060 (Sam's good old *fun days) , Sam's your man here Raj. 2) i think that any extra/redundant css might be ok to clean up in master - but i'm not sure about stability issues for themes , again Sam's the man. oh, He's also the integrator here cheers!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ho:

          • I think it's ok to backport this (as far as it's an accessibility bugfix).
          • Also, I think the new progressbar implementation is better than the old one. Not sure if that must be considered an UI change, from a functional POV, the results are the same, no matter the underlying implementation.
          • But, in the new issue (to be created!), please look at all files having progress_bar_(token|table), there are occurrences also within theme/standard/style/modules.css. And also, propose to test it under some more themes, just in case something became borked on any of them.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Ho: I think it's ok to backport this (as far as it's an accessibility bugfix). Also, I think the new progressbar implementation is better than the old one. Not sure if that must be considered an UI change, from a functional POV, the results are the same, no matter the underlying implementation. But, in the new issue (to be created!), please look at all files having progress_bar_(token|table), there are occurrences also within theme/standard/style/modules.css. And also, propose to test it under some more themes, just in case something became borked on any of them. Ciao
          Hide
          Helen Foster added a comment -

          Hi Rosie,

          Even with a couple of questions in the lesson and attempting it as a student, I still couldn't see a progress bar. I discovered that the problem has already been reported as MDL-26016.

          Raj, did you actually see the progress bar when you tested the issue or just a green box?

          Show
          Helen Foster added a comment - Hi Rosie, Even with a couple of questions in the lesson and attempting it as a student, I still couldn't see a progress bar. I discovered that the problem has already been reported as MDL-26016 . Raj, did you actually see the progress bar when you tested the issue or just a green box?
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Some changes to Moodle should be milestones in the project by themselves.

          This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks!

          Closing as fixed, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Some changes to Moodle should be milestones in the project by themselves. This is not the case and your fix is not so important, but your collaboration is highly appreciated, thanks! Closing as fixed, ciao

            People

            • Votes:
              1 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: