Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.3
    • Fix Version/s: 2.3
    • Component/s: Themes
    • Labels:
    • Rank:
      40203

      Description

      In afterburner .headingblock is hidden. So replace .headingblock with section heading block. This will give more flexibility to theme designer's.
      Also, upgrade base version.

      1. after.png
        22 kB
      2. before.png
        22 kB

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Perhaps Mary should be involved in this.

          Could you also elaborate on your description?

          Show
          Michael de Raadt added a comment - Perhaps Mary should be involved in this. Could you also elaborate on your description?
          Show
          Rajesh Taneja added a comment - Hello Michael, This was found while testing MDL-32770 . More info is available in following comments: http://tracker.moodle.org/browse/MDL-32770?focusedCommentId=157746&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-157746 and http://tracker.moodle.org/browse/MDL-32770?focusedCommentId=157747&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-157747
          Hide
          Mary Evans added a comment - - edited

          Hello everyone,
          Did I hear my name mentioned?

          Rajesh, if this is just an Afterburner bug related issue I can fix it. Shall I assign it to me?

          The course headingblock was disabled because of the hundreds of requests from Moodlers to either change Topic outline or Weekly outline, or hide them. Which is I think what you are asking me to reverse?

          I have not have time to check this out...so this is just pure guess work at this time of the morning, but I am certain this is what it is. Since you are the first to ask about this, I am certain no one has missed it that particular heading.

          That said is there any way that Course writers (Admins/Editing Teachers) can enter a custom name for "Topic outline" or "Weekly outline" when creating a course? This has been asked for time and time again. If you could suggest a fix for this I would be quite willing to do the work.

          Thanks
          Mary

          Show
          Mary Evans added a comment - - edited Hello everyone, Did I hear my name mentioned? Rajesh, if this is just an Afterburner bug related issue I can fix it. Shall I assign it to me? The course headingblock was disabled because of the hundreds of requests from Moodlers to either change Topic outline or Weekly outline, or hide them. Which is I think what you are asking me to reverse? I have not have time to check this out...so this is just pure guess work at this time of the morning, but I am certain this is what it is. Since you are the first to ask about this, I am certain no one has missed it that particular heading. That said is there any way that Course writers (Admins/Editing Teachers) can enter a custom name for "Topic outline" or "Weekly outline" when creating a course? This has been asked for time and time again. If you could suggest a fix for this I would be quite willing to do the work. Thanks Mary
          Hide
          Mary Evans added a comment -

          @Gareth I've just added you as a watch to this track issue which you may like to comment about if you read the discussion.

          Thanks
          Mary

          Show
          Mary Evans added a comment - @Gareth I've just added you as a watch to this track issue which you may like to comment about if you read the discussion. Thanks Mary
          Hide
          Rajesh Taneja added a comment -

          Thanks for the feedback, Mary.

          In 2.3 we have introduce section view. While testing MDL-32770, we realised headingblock is hidden in afterburner theme only.
          It will be very helpful, if you can please grab patch from MDL-32770, and provide some feedback.

          I have fixed this issue for now and tested it on most themes. I will be happy to have more feedback, to avoid any theme regressions.

          Show
          Rajesh Taneja added a comment - Thanks for the feedback, Mary. In 2.3 we have introduce section view. While testing MDL-32770 , we realised headingblock is hidden in afterburner theme only. It will be very helpful, if you can please grab patch from MDL-32770 , and provide some feedback. I have fixed this issue for now and tested it on most themes. I will be happy to have more feedback, to avoid any theme regressions.
          Show
          Rajesh Taneja added a comment - Please pull last two commits from this branch. https://github.com/rajeshtaneja/moodle/commit/4f82488932e67c30ba09982f85c0bdd8b7a558e8 and https://github.com/rajeshtaneja/moodle/commit/8ea356304b6350f6eeea72ad0f286a5b652b3b35
          Hide
          Gareth J Barnard added a comment -

          @Mary Thanks - interesting!

          The changes for Moodle 2.3 are certainly different - I've raised CONTRIB-3652 on myself to handle it.

          To change the wording of "Topic outline" or "Weekly outline" this can be done in the standard language file editing functionality but will affect all courses as the code uses 'get_string('topicoutline')' to get the words.

          I also think that there is now getting to be a connection between themes and course formats in terms of being able to manipulate on a course by course basis the layout of the page that you want. I know that Wordpress allows you to set the layout of the whole page depending on the type of content being presented. So why not provide that base functionality in themes, set on a course by course basis and let course formats control the middle section or even merge the two pieces of functionality together can call it 'course layout' as a sub-element of an overall theme implementation.

          Cheers,

          Gareth

          Show
          Gareth J Barnard added a comment - @Mary Thanks - interesting! The changes for Moodle 2.3 are certainly different - I've raised CONTRIB-3652 on myself to handle it. To change the wording of "Topic outline" or "Weekly outline" this can be done in the standard language file editing functionality but will affect all courses as the code uses 'get_string('topicoutline')' to get the words. I also think that there is now getting to be a connection between themes and course formats in terms of being able to manipulate on a course by course basis the layout of the page that you want. I know that Wordpress allows you to set the layout of the whole page depending on the type of content being presented. So why not provide that base functionality in themes, set on a course by course basis and let course formats control the middle section or even merge the two pieces of functionality together can call it 'course layout' as a sub-element of an overall theme implementation. Cheers, Gareth
          Hide
          Gareth J Barnard added a comment -

          Also noticed that you can now set the theme on a course by course basis 'http://docs.moodle.org/22/en/admin/setting/themesettings' so could have different 'layouts'.

          Show
          Gareth J Barnard added a comment - Also noticed that you can now set the theme on a course by course basis 'http://docs.moodle.org/22/en/admin/setting/themesettings' so could have different 'layouts'.
          Hide
          Mary Evans added a comment -

          Thansk for that Gareth.

          And I have just found that each course format carriers a renderers.php which is fantasting news!

          Show
          Mary Evans added a comment - Thansk for that Gareth. And I have just found that each course format carriers a renderers.php which is fantasting news!
          Hide
          Gareth J Barnard added a comment -

          @Mary - No worries. I suppose that it is good news in terms of technology and using polymorphism - but it means more work for me just to stand still! BTW - You might be interested in this - http://moodle.org/mod/forum/discuss.php?d=202843 - discussion about the MyMobile theme and my format.

          Show
          Gareth J Barnard added a comment - @Mary - No worries. I suppose that it is good news in terms of technology and using polymorphism - but it means more work for me just to stand still! BTW - You might be interested in this - http://moodle.org/mod/forum/discuss.php?d=202843 - discussion about the MyMobile theme and my format.
          Hide
          Rajesh Taneja added a comment -

          Moving it to "Must fix in 2.3"

          Show
          Rajesh Taneja added a comment - Moving it to "Must fix in 2.3"
          Hide
          Mary Evans added a comment - - edited

          Rajesh,

          I tested this last night on the current Moodle 2.3(ALPHA) via CSV. I see what you mean now with Afterburner.

          I'm not 100% sure if I tested it correctly, as these commits would not yet be in Moodle 2.3 of the version I used.

          Cheers
          Mary

          Show
          Mary Evans added a comment - - edited Rajesh, I tested this last night on the current Moodle 2.3(ALPHA) via CSV. I see what you mean now with Afterburner. I'm not 100% sure if I tested it correctly, as these commits would not yet be in Moodle 2.3 of the version I used. Cheers Mary
          Hide
          Rajesh Taneja added a comment -

          Thanks Mary,

          It can be helpful, if you can look at the attached patch and see if it make sense.

          Show
          Rajesh Taneja added a comment - Thanks Mary, It can be helpful, if you can look at the attached patch and see if it make sense.
          Hide
          Mary Evans added a comment -

          Rajesh I have just been testing this in Afterburner and it seems to work OK. In fact it's looking good.

          Cheers
          Mary

          Show
          Mary Evans added a comment - Rajesh I have just been testing this in Afterburner and it seems to work OK. In fact it's looking good. Cheers Mary
          Hide
          Rajesh Taneja added a comment -

          Thanks Mary,

          Pushing it for integration review

          Show
          Rajesh Taneja added a comment - Thanks Mary, Pushing it for integration review
          Hide
          Dan Poltawski added a comment -

          Hi Raj,

          1. We seem to have lost a border around the header which was giving some useful distinction to it - see before vs after screenshots i've uploaded
          2. You have named two attributes arrays $dimlink, that naming gives the impression that we are always dimming the link, where as we are conditionally. I think it'd be less easy to get confused if they were not named like that. (e.g. attributes)
          3. Please just append the 'dimmed_text' here, then there is less chance of inconsistency between dimmed and non-dimmed titles:
            $titleattr = array('class' => 'mdl-align title');
            if (!$sections[$displaysection]->visible) {
               $titleattr = array('class' => 'mdl-align title dimmed_text');
            }
            
          Show
          Dan Poltawski added a comment - Hi Raj, We seem to have lost a border around the header which was giving some useful distinction to it - see before vs after screenshots i've uploaded You have named two attributes arrays $dimlink, that naming gives the impression that we are always dimming the link, where as we are conditionally. I think it'd be less easy to get confused if they were not named like that. (e.g. attributes) Please just append the 'dimmed_text' here, then there is less chance of inconsistency between dimmed and non-dimmed titles: $titleattr = array('class' => 'mdl-align title'); if (!$sections[$displaysection]->visible) { $titleattr = array('class' => 'mdl-align title dimmed_text'); }
          Hide
          Rajesh Taneja added a comment -

          Thanks Dan,

          I will fix this.

          Show
          Rajesh Taneja added a comment - Thanks Dan, I will fix this.
          Hide
          Rajesh Taneja added a comment -

          I have taken care of all recommendations.
          Hope this is fine now.

          Show
          Rajesh Taneja added a comment - I have taken care of all recommendations. Hope this is fine now.
          Hide
          Dan Poltawski added a comment -

          Thanks Raj, i've integrated that now

          Show
          Dan Poltawski added a comment - Thanks Raj, i've integrated that now
          Hide
          Frédéric Massart added a comment -

          Test successful

          Show
          Frédéric Massart added a comment - Test successful
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: