Details

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

      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.

        Gliffy Diagrams

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

          Issue Links

            Activity

            Hide
            salvetore Michael de Raadt added a comment -

            Perhaps Mary should be involved in this.

            Could you also elaborate on your description?

            Show
            salvetore Michael de Raadt added a comment - Perhaps Mary should be involved in this. Could you also elaborate on your description?
            Show
            rajeshtaneja 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
            lazydaisy 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
            lazydaisy 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
            lazydaisy 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
            lazydaisy 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
            rajeshtaneja 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
            rajeshtaneja 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
            rajeshtaneja 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
            gb2048 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
            gb2048 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
            gb2048 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
            gb2048 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
            lazydaisy 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
            lazydaisy 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
            gb2048 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
            gb2048 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
            rajeshtaneja Rajesh Taneja added a comment -

            Moving it to "Must fix in 2.3"

            Show
            rajeshtaneja Rajesh Taneja added a comment - Moving it to "Must fix in 2.3"
            Hide
            lazydaisy 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
            lazydaisy 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
            rajeshtaneja 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
            rajeshtaneja 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
            lazydaisy 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
            lazydaisy 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
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Mary,

            Pushing it for integration review

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Mary, Pushing it for integration review
            Hide
            poltawski 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
            poltawski 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
            rajeshtaneja Rajesh Taneja added a comment -

            Thanks Dan,

            I will fix this.

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

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

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

            Thanks Raj, i've integrated that now

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

            Test successful

            Show
            fred Frédéric Massart added a comment - Test successful
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  25/Jun/12