Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-33027

Improve section navigation css

    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

          Attachments

            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