Moodle
  1. Moodle
  2. MDL-35816 Accessibility Review issues (Deque)
  3. MDL-36104

Assignment grading feedback comments text area has no label and a blank label on the form

    Details

    • Testing Instructions:
      Hide
      1. Log in as an administrator and create a course.
      2. Create an assignment in this course with the settings 'Online text', 'File submissions', 'Feedback comments' and 'Feedback files' set to yes.
      3. Login as a student and visit the assignment submission page.
      4. View the HTML source and ensure there is a hidden label next to the html editor under 'Online text' with the same value, do the same for 'File submissions'.
      5. Log back in as the administrator and navigate to the assignment grading for that individual submission.
      6. View the HTML source and ensure there is a hidden label next to the html editor under 'Feedback comments' with the same value, do the same for 'Feedback files'.
      Show
      Log in as an administrator and create a course. Create an assignment in this course with the settings 'Online text', 'File submissions', 'Feedback comments' and 'Feedback files' set to yes. Login as a student and visit the assignment submission page. View the HTML source and ensure there is a hidden label next to the html editor under 'Online text' with the same value, do the same for 'File submissions'. Log back in as the administrator and navigate to the assignment grading for that individual submission. View the HTML source and ensure there is a hidden label next to the html editor under 'Feedback comments' with the same value, do the same for 'Feedback files'.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-36104_m24
    • Pull Master Branch:
      MDL-36104_master
    • Rank:
      44869

      Description

      Issue
      Explicit labels - The label for the feedback comments section of the assignment grading screen is empty and therefore the feedback comments text area has no label.

      Standard Level
      WCAG 2 1.1.1 (A) []

      Impact
      Serious

      Example Link
      http://demo.moodle.net/

      Test Steps

      1. Login as teacher
      2. Access CF101
      3. Create assignment with feedback comments set to yes
      4. Save and display
      5. Login as student and submit an assignment to the course
      6. Log back in as teacher and navigate to the assignment grading
      7. View the source of the feedback comments area.

        Activity

        Hide
        Helen Foster added a comment -

        Just attaching a screenshot of the assignment grading page feedback comments area and a screenshot of a similar situation elsewhere in Moodle - the quiz settings page.

        In the quiz settings, the text area is labelled 'Feedback' and the box around is called 'Overall feedback'.

        To avoid having 'Feedback comments' displayed twice in the assignment grading page, I suggest that the text area is labelled 'Feedback' and the box around is left as 'Feedback comments'.

        Show
        Helen Foster added a comment - Just attaching a screenshot of the assignment grading page feedback comments area and a screenshot of a similar situation elsewhere in Moodle - the quiz settings page. In the quiz settings, the text area is labelled 'Feedback' and the box around is called 'Overall feedback'. To avoid having 'Feedback comments' displayed twice in the assignment grading page, I suggest that the text area is labelled 'Feedback' and the box around is left as 'Feedback comments'.
        Hide
        Mark Nelson added a comment -

        Thanks Helen.

        Show
        Mark Nelson added a comment - Thanks Helen.
        Hide
        Jason Fowler added a comment -

        you can also try doing adding a label via mforms. I think I pulled it off for this issue: https://tracker.moodle.org/browse/MDL-35874

        Show
        Jason Fowler added a comment - you can also try doing adding a label via mforms. I think I pulled it off for this issue: https://tracker.moodle.org/browse/MDL-35874
        Hide
        Mark Nelson added a comment -

        Hi Jason, I looked at your commit and see that it is not using mforms. It is using the set_label function for the class single_select which I knew existed, but can't use in this case. I do not think it is possible to set a class on a label using mforms without a drastic change in the mform api, so I am leaving as it is since this is what other modules are doing. Can you peer review what I have atm and push to integration when done? Thanks.

        Show
        Mark Nelson added a comment - Hi Jason, I looked at your commit and see that it is not using mforms. It is using the set_label function for the class single_select which I knew existed, but can't use in this case. I do not think it is possible to set a class on a label using mforms without a drastic change in the mform api, so I am leaving as it is since this is what other modules are doing. Can you peer review what I have atm and push to integration when done? Thanks.
        Hide
        Jason Fowler added a comment -

        [Y] Syntax
        [Y] Output
        [Y] Whitespace
        [Y] Language
        [Y] Databases
        [Y] Testing
        [-] Security
        [-] Documentation
        [Y] Git
        [Y] Sanity check

        Just wondering if "Feedback" is the idea text for this. Maybe "Feedback comments" might be better. What ever it is, it needs to be consistent with the other aspects of moodle - quiz etc.

        Show
        Jason Fowler added a comment - [Y] Syntax [Y] Output [Y] Whitespace [Y] Language [Y] Databases [Y] Testing [-] Security [-] Documentation [Y] Git [Y] Sanity check Just wondering if "Feedback" is the idea text for this. Maybe "Feedback comments" might be better. What ever it is, it needs to be consistent with the other aspects of moodle - quiz etc.
        Hide
        Mark Nelson added a comment -

        Spoken to Jason, it's fine as the images provided by Helen show a similar situation. Submitting for integration.

        Show
        Mark Nelson added a comment - Spoken to Jason, it's fine as the images provided by Helen show a similar situation. Submitting for integration.
        Hide
        Dan Poltawski 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
        Dan Poltawski 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 -

        Adding Damyon here as component maintainer, please make sure you involve the component maintainer when making changes to their component.

        Damyon, can you give this a +1 if you are happy with it?

        Show
        Dan Poltawski added a comment - Adding Damyon here as component maintainer, please make sure you involve the component maintainer when making changes to their component. Damyon, can you give this a +1 if you are happy with it?
        Hide
        Damyon Wiese added a comment -

        Thanks Dan this gets a -1 from me.

        All the other feedback/submission plugins do the same thing - so changing only one is in-consistent for a start.

        Second the label too generic and adds no useful information. If we did the same for all plugins we would just get "Feedback", "Feedback", "Feedback" all the way down the page.

        Perhaps this is one to raise on Thursday. (Is it ok to add a label and hide it with CSS ?)

        Show
        Damyon Wiese added a comment - Thanks Dan this gets a -1 from me. All the other feedback/submission plugins do the same thing - so changing only one is in-consistent for a start. Second the label too generic and adds no useful information. If we did the same for all plugins we would just get "Feedback", "Feedback", "Feedback" all the way down the page. Perhaps this is one to raise on Thursday. (Is it ok to add a label and hide it with CSS ?)
        Hide
        Jason Fowler added a comment -

        Damyon, we tried looking in to hiding it with CSS, and while it can be done, it's kind of hacky - mforms is a pain to work with on things like this, and it would require placing a span inside the label, then hiding the span, as the label can't be given a class directly.

        I think all the other feedback/submission plugins need to fall in line with what has been put forward here, rather than holding this back because the others don't do it.

        Show
        Jason Fowler added a comment - Damyon, we tried looking in to hiding it with CSS, and while it can be done, it's kind of hacky - mforms is a pain to work with on things like this, and it would require placing a span inside the label, then hiding the span, as the label can't be given a class directly. I think all the other feedback/submission plugins need to fall in line with what has been put forward here, rather than holding this back because the others don't do it.
        Hide
        Damyon Wiese added a comment -

        Yes - talked with Mark and I am fine if we add a label for all the plugins at once (but the label should be the name of the plugin - not just "Feedback" or "Submission") as there may be multiple on the page at once.

        Show
        Damyon Wiese added a comment - Yes - talked with Mark and I am fine if we add a label for all the plugins at once (but the label should be the name of the plugin - not just "Feedback" or "Submission") as there may be multiple on the page at once.
        Hide
        Mark Nelson added a comment -

        It looks odd if the label and the header contain the same text, so rather than display the label I have chosen to wrap it in a span tag using the CSS class accesshide. Submitting back for peer.

        Show
        Mark Nelson added a comment - It looks odd if the label and the header contain the same text, so rather than display the label I have chosen to wrap it in a span tag using the CSS class accesshide. Submitting back for peer.
        Hide
        Damyon Wiese added a comment -

        This looks good Mark, but can we do the same for mod/assign/submission/onlinetext/locallib.php and mod/assign/submission/file/locallib.php ?

        Thanks Mark

        Show
        Damyon Wiese added a comment - This looks good Mark, but can we do the same for mod/assign/submission/onlinetext/locallib.php and mod/assign/submission/file/locallib.php ? Thanks Mark
        Hide
        Mark Nelson added a comment -

        Hi Damyon, done. Please review again. Thanks.

        Show
        Mark Nelson added a comment - Hi Damyon, done. Please review again. Thanks.
        Hide
        Damyon Wiese added a comment -

        Thanks Mark - all good now.

        +1 from me.

        Sending for integration.

        Show
        Damyon Wiese added a comment - Thanks Mark - all good now. +1 from me. Sending for integration.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Integrated (23, 24 & master), thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - Integrated (23, 24 & master), thanks!
        Hide
        Frédéric Massart added a comment -

        Passed on 2.3 onwards. Thanks!

        Show
        Frédéric Massart added a comment - Passed on 2.3 onwards. Thanks!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks!

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Surely you will be happy to know that your code is now part of Moodle upstream. Thanks, thanks! Closing as fixed, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: