Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.0.3
    • Fix Version/s: 2.4
    • Component/s: Accessibility, Calendar
    • Labels:
    • Testing Instructions:
      Hide
      1. goto calendar and click on new event
      2. make sure the form is divided into three fieldsets with legends "general" , "duration", "repeated events" respectively
      3. verify the same for the form to edit a repeated event
      4. Verify the form to edit a non-repeated element has two fieldset with legend "general" and "duration"
      Show
      goto calendar and click on new event make sure the form is divided into three fieldsets with legends "general" , "duration", "repeated events" respectively verify the same for the form to edit a repeated event Verify the form to edit a non-repeated element has two fieldset with legend "general" and "duration"
    • Affected Branches:
      MOODLE_20_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-28071-master

      Description

      • the label on 'date' does not have 'for' attribute and the fieldset around the date controls does not have a legend. I think it would be preferable if the word 'date' was the legend for the fieldset.
      • there are two instances of <label for="id_e3062b"> on 'duration' but as I understand it these should be unique. Same applies to 'until', 'Duration in minutes', 'Repeat this event' and 'Repeat weekly, creating altogether', and 'save changes'.
      • There should be a fieldset for the duration radio buttons and the word 'duration' should be the legend for the fieldset.
      • there is an empty label tag after 'until' and before 'day' (this may be because day is currently greyed out).
      • The whole form is within a fieldset which does not have a legend, but I would not normally suggest placing a whole form within this tag.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Ankit Agarwal added a comment -

            This patch :-

            • Does not fix 'label on "date"' issue. Please refer MDL-30892 for further discussion
            • All multiple labels are with radio buttons needs change in a behavior of radio elements. Discussed this with Dan, and Raj and we agreed to wait on the major form improvements to fix this. (Refer MDL-35559)
            • All duration elements are now in a fieldset
            • All repeat elements are now in a fieldset
            • Repeat this event and 'Repeat weekly, creating altogether', and 'save changes' doesn't have multiple labels.
            • Adds a legend to the form field-set (Hence the class maincalendar is removed).

            Also

            • Added ui_change label
            • Added testing instructions
            • Not sure if this should go in stable as there are some major ui_changes (will confirm from integrators)

            Requesting a review.
            Thanks

            Show
            Ankit Agarwal added a comment - This patch :- Does not fix 'label on "date"' issue. Please refer MDL-30892 for further discussion All multiple labels are with radio buttons needs change in a behavior of radio elements. Discussed this with Dan, and Raj and we agreed to wait on the major form improvements to fix this. (Refer MDL-35559 ) All duration elements are now in a fieldset All repeat elements are now in a fieldset Repeat this event and 'Repeat weekly, creating altogether', and 'save changes' doesn't have multiple labels. Adds a legend to the form field-set (Hence the class maincalendar is removed). Also Added ui_change label Added testing instructions Not sure if this should go in stable as there are some major ui_changes (will confirm from integrators) Requesting a review. Thanks
            Hide
            Frédéric Massart added a comment - - edited

            Hi Ankit,

            those changes make sense, and considering your other comments I don't think this could be improved more within the scope of this issue.

            My +1 for integration crash and burn!

            (Edit: You didn't mention the duplicated IDs for the div encapsulating each radio elements, but this should be fixed in the another issue IMO.)

            Show
            Frédéric Massart added a comment - - edited Hi Ankit, those changes make sense, and considering your other comments I don't think this could be improved more within the scope of this issue. My +1 for integration crash and burn! (Edit: You didn't mention the duplicated IDs for the div encapsulating each radio elements, but this should be fixed in the another issue IMO.)
            Hide
            Ankit Agarwal added a comment - - edited

            Thanks for review Fred.
            Duplicate id issue should be taken care in MDL-35559. I have commented in there with a few other stuff.
            Submitting only master only as per Dan's advice.

            Thanks

            Show
            Ankit Agarwal added a comment - - edited Thanks for review Fred. Duplicate id issue should be taken care in MDL-35559 . I have commented in there with a few other stuff. Submitting only master only as per Dan's advice. Thanks
            Hide
            Sam Hemelryk added a comment -

            Looks good thanks Ankit, will be integrated as soon as I have access to git.moodle.org again

            Show
            Sam Hemelryk added a comment - Looks good thanks Ankit, will be integrated as soon as I have access to git.moodle.org again
            Hide
            Sam Hemelryk added a comment -

            Integrated now thanks

            Show
            Sam Hemelryk added a comment - Integrated now thanks
            Hide
            Jason Fowler added a comment -

            Looks great Ankit

            Show
            Jason Fowler added a comment - Looks great Ankit
            Hide
            Dan Poltawski added a comment -

            Congratulations, you've done it!

            Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc

            Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

            Show
            Dan Poltawski added a comment - Congratulations, you've done it! Nf n erjneq sbe fhpprfshy vagrtengvba vagb guvf jrrxf eryrnfr, V pna abj qvfpybfr gb lbh gur rkvfgnapr bs shapgvba fge_ebg13(), gb tb va lbhe gbbyxvg nybat jvgu uggc://cuc.arg/znahny/ra/shapgvba.tmtrgff.cuc Cyrnfr qb abg nyybj guvf vasbezngvba gb cnff shegure.

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: