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
    • Rank:
      16854

      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.

        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: