Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3.1
    • Fix Version/s: 2.4
    • Component/s: Accessibility, Calendar
    • Labels:
    • Testing Instructions:
      Hide
      1. Goto calendar monthly view.
      2. Make sure the title of page is something like "stable_master: Calendar: Detailed month view :September 2012"
      3. click on any date that has event
      4. Make sure the title of the page is like "stable_master: Calendar: Day view :Tuesday, 4 September 2012"
      5. Goto upcoming events page (add a upcoming event block to get the link)
      6. Check the title of the page is like "stable_master: Calendar: Upcoming events"
      Show
      Goto calendar monthly view. Make sure the title of page is something like "stable_master: Calendar: Detailed month view :September 2012" click on any date that has event Make sure the title of the page is like "stable_master: Calendar: Day view :Tuesday, 4 September 2012" Goto upcoming events page (add a upcoming event block to get the link) Check the title of the page is like "stable_master: Calendar: Upcoming events"
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      MDL-35224-master
    • Rank:
      43868

      Description

      Multipage items (e.g. Book, Activity Reports, Calendar) should have their page titles change to indicate to the user which page of a resource they are viewing instead of always keeping the same title.

      Greg's comment on MDL-30896 suggest
      For the calendar

      calendar/view.php?view=month&course=1&cal_d=1&cal_m=1&cal_y=2012

      when I view January the page title should be something like

      "moodle-access: Calendar: Detailed month view: January 2012"

      instead of every page just saying

      "moodle-access: Calendar: Detailed month view"

      That way when I go to February the page title changes accordingly.

        Activity

        Hide
        Ankit Agarwal added a comment -

        Will update stable branches after the review.
        Thanks

        Show
        Ankit Agarwal added a comment - Will update stable branches after the review. Thanks
        Hide
        Rossiani Wijaya added a comment -

        Hi Ankit,

        Just a matter of preference, maybe it is better to use '-' instead of ':' for the new string concatenation. (eg: calendaer - day view).

        I will leave it to you for the decision.

        Other than that, it looks good.

        Show
        Rossiani Wijaya added a comment - Hi Ankit, Just a matter of preference, maybe it is better to use '-' instead of ':' for the new string concatenation. (eg: calendaer - day view). I will leave it to you for the decision. Other than that, it looks good.
        Hide
        Ankit Agarwal added a comment -

        Hi Rosie,
        Thanks for your feedback.
        from what I have seen in rest of the code and around here as well we use ":" to concatenate strings in title.

        Thanks

        Show
        Ankit Agarwal added a comment - Hi Rosie, Thanks for your feedback. from what I have seen in rest of the code and around here as well we use ":" to concatenate strings in title. Thanks
        Hide
        Ankit Agarwal added a comment -

        sending for integration.
        Thanks

        Show
        Ankit Agarwal added a comment - sending for integration. 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
        Ankit Agarwal added a comment -

        rebased
        Thanks

        Show
        Ankit Agarwal added a comment - rebased Thanks
        Hide
        Dan Poltawski added a comment -

        Sorry Ankit, I don't like this.

        1. $accessiblitystr should be initialised before use (like $pagetitle)
        2. $accessiblitystr is a bad name. This isn't accessibility specific, it isn't accessibility specific information and in fact its useful for everyone.
        3. I think a better soltuion would be to change the title string and include placeholder for the date information so that it can be properly localised.
        4. Note this suggestion with strings may make this solution only suitable for master (see also my comments on MDL-35225)
        Show
        Dan Poltawski added a comment - Sorry Ankit, I don't like this. $accessiblitystr should be initialised before use (like $pagetitle) $accessiblitystr is a bad name. This isn't accessibility specific, it isn't accessibility specific information and in fact its useful for everyone. I think a better soltuion would be to change the title string and include placeholder for the date information so that it can be properly localised. Note this suggestion with strings may make this solution only suitable for master (see also my comments on MDL-35225 )
        Hide
        Ankit Agarwal added a comment -

        Hi Dan,
        We cannot change the title strings as they are used in multiple places. We can introduce new strings with placeholders, but for the same reasons you mentioned in MDL-35225 am not sure it's a good idea to increase the load of translators when it is not required.

        I will make the first two changes you mentioned. Let me know if you want new strings being introduced.

        Thanks

        Show
        Ankit Agarwal added a comment - Hi Dan, We cannot change the title strings as they are used in multiple places. We can introduce new strings with placeholders, but for the same reasons you mentioned in MDL-35225 am not sure it's a good idea to increase the load of translators when it is not required. I will make the first two changes you mentioned. Let me know if you want new strings being introduced. Thanks
        Hide
        Dan Poltawski added a comment -

        Yes, IMO a better title string with placeholders is worth it for master.

        Show
        Dan Poltawski added a comment - Yes, IMO a better title string with placeholders is worth it for master.
        Hide
        Ankit Agarwal added a comment -

        Update master branch, Removed stables.
        Thanks

        Show
        Ankit Agarwal added a comment - Update master branch, Removed stables. Thanks
        Hide
        Dan Poltawski added a comment -

        " 'Detailed month view :{$a}';"

        Show
        Dan Poltawski added a comment - " 'Detailed month view :{$a}';"
        Hide
        Dan Poltawski added a comment -

        Thanks Ankit.

        Show
        Dan Poltawski added a comment - Thanks Ankit.
        Hide
        Frédéric Massart added a comment -

        Perfect, passing!

        Show
        Frédéric Massart added a comment - Perfect, passing!
        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!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: