Moodle
  1. Moodle
  2. MDL-18971

styles_color.css problem effecting forum unread post highlighting

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.9.4
    • Fix Version/s: 1.9.6
    • Component/s: Forum, Themes
    • Labels:
      None
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_19_STABLE
    • Rank:
      31754

      Description

      The standard styles_color.css contains an ".unread" class and a number of classes specific to the forum (see http://docs.moodle.org/en/CSS_styles_color.css#Forum).

      A forum main page uses the ".unread" class to specify the style for the values in the unread column.

      Pages displaying posts...
      *in threaded view use the class "forumthread unread" to specify the style for unread headers
      *in nested view use the class "forumpost unread" to specify the style for unread posts

      In styles_color.css the "forumthread unread" class is specified as:
      .forumthread .unread {
      background: #FFD991;
      }

      There is no specification for the "forumpost unread" class, which in itself is not a problem. However, there is no specification for either ".forumthread" or ".forumpost".

      Combining two classes together (e.g. as done above with .forumthread .unread) adds nothing to the style (i.e. the "background: #FFD991;" is ignored). The only reason the "forumthread unread" and "forumpost unread" classes work is because there is an ".unread" class specified in styles_color.css. This results in unread posts failing to be highlighted in various themes that do not include the styles_color.css or a specific ".unread" class (examples include formal_white, chameleon, etc.).

      Moodle.org forums are exhibiting a problem with highlighting unread posts where the nested view correctly differentiates between read and unread posts but the threaded view highlights everything as unread. I believe this problem is tied to the coding of the CSS associated with "forumthread unread" and "forumthread read" described above.

      Combining two classes together is done because you want the style from both to be applied. There is no need to specify a combined class in a CSS unless you desire it for a placeholder.

      Proposed Solution:
      The ".unread" class needs to be a part of every Moodle provided theme. Either include it in *_color.css or include styles_color.css in the standardsheets array in config.php.

      In the forum section of styles_color.css all of the classes need to be specified before they are combined with other classes (e.g. forumpost and forumthread).

      I am including a test file that illustrates behavior from combining classes in case anyone wants to experiment with the underlying problem identified here.

      1. MDL-18971.patch
        0.9 kB
        Dongsheng Cai
      2. MDL-18971v2.patch
        3 kB
        Eric Straw
      3. MDL-18971v3.patch
        9 kB
        Eric Straw
      4. test.html
        0.8 kB
        Eric Straw

        Activity

        Hide
        Dongsheng Cai added a comment -

        Hi, Eric
        .forumthread .unread

        { background: #FFD991; }


        This is not a combination, it actually defines a unread class inside .forumthread class, this is incorrect, thanks for reporting.

        Show
        Dongsheng Cai added a comment - Hi, Eric .forumthread .unread { background: #FFD991; } This is not a combination, it actually defines a unread class inside .forumthread class, this is incorrect, thanks for reporting.
        Hide
        Dongsheng Cai added a comment -

        I propose a patch here, can you have a look, Eric?

        Show
        Dongsheng Cai added a comment - I propose a patch here, can you have a look, Eric?
        Hide
        Dongsheng Cai added a comment -

        Any comments?

        Show
        Dongsheng Cai added a comment - Any comments?
        Hide
        Dongsheng Cai added a comment -

        Fixed, please review.

        Show
        Dongsheng Cai added a comment - Fixed, please review.
        Hide
        Eric Straw added a comment -

        Dongsheng,
        Sorry about the delay in my response.

        I tested the patch in the following manner:
        1)Clean install of Moodle
        2)Applied patch
        3)Tested using the standard theme – highlighting seems to work like it should.
        4)Modified each .unread background color to a unique value so I could see the difference. The only background color that shows is the value in .unread from the core section. The patched lines of code make no difference in the background color.

        Also, this patch does nothing to solve the problem in other themes provided with Moodle (e.g. I applied the formal_white theme – highlighting does not work ).

        Conclusion:
        This patch does not adequately address the problem.

        I'll start working on a possible patch to see what I can come up with.

        Show
        Eric Straw added a comment - Dongsheng, Sorry about the delay in my response. I tested the patch in the following manner: 1)Clean install of Moodle 2)Applied patch 3)Tested using the standard theme – highlighting seems to work like it should. 4)Modified each .unread background color to a unique value so I could see the difference. The only background color that shows is the value in .unread from the core section. The patched lines of code make no difference in the background color. Also, this patch does nothing to solve the problem in other themes provided with Moodle (e.g. I applied the formal_white theme – highlighting does not work ). Conclusion: This patch does not adequately address the problem. I'll start working on a possible patch to see what I can come up with.
        Hide
        Eric Straw added a comment -

        This patch includes...
        *comments for all forum specific classes.
        *highlighting that works independently for each use of the .unread class (i.e. you can set the colors differently and they will properly display)

        To be fully effective this patch needs to be incorporated in to every theme provided with Moodle in one of two ways:
        1)Include styles_color.css in the standardsheets array in each themes config.php
        2)Patch the styles_color.css in each theme that includes its own version of that file

        Show
        Eric Straw added a comment - This patch includes... *comments for all forum specific classes. *highlighting that works independently for each use of the .unread class (i.e. you can set the colors differently and they will properly display) To be fully effective this patch needs to be incorporated in to every theme provided with Moodle in one of two ways: 1)Include styles_color.css in the standardsheets array in each themes config.php 2)Patch the styles_color.css in each theme that includes its own version of that file
        Hide
        Eric Straw added a comment -

        The above comment was referring to MDL-18971v2.patch.

        Let me know how the patch tests for you Dongsheng.

        Show
        Eric Straw added a comment - The above comment was referring to MDL-18971 v2.patch. Let me know how the patch tests for you Dongsheng.
        Hide
        Eric Straw added a comment -

        v3 is the complete fix. The patch was developed on 1.9.5 build 20090513.

        standard/styles_color.css contains the proper classes. The unread background or border color for each of the following pages can be independently modified:
        course/view.php
        mod/forum/index.php
        mod/forum/view.php
        mod/forum/discuss.php

        On mod/forum/discuss.php there are independent classes for nested and threaded views.

        Modifies the following in /moodle/theme/:
        chameleon/config.php
        formal_white/config.php
        formal_white/fw_color.css
        metal/colors.css
        oceanblue/styles_color.css
        orangewhite/config.php
        orangewhite/styles_color.css
        orangewhitepda/config.php
        standard/styles_color.css

        Show
        Eric Straw added a comment - v3 is the complete fix. The patch was developed on 1.9.5 build 20090513. standard/styles_color.css contains the proper classes. The unread background or border color for each of the following pages can be independently modified: course/view.php mod/forum/index.php mod/forum/view.php mod/forum/discuss.php On mod/forum/discuss.php there are independent classes for nested and threaded views. Modifies the following in /moodle/theme/: chameleon/config.php formal_white/config.php formal_white/fw_color.css metal/colors.css oceanblue/styles_color.css orangewhite/config.php orangewhite/styles_color.css orangewhitepda/config.php standard/styles_color.css
        Hide
        Eric Straw added a comment -

        Dongsheng,
        When I installed 1.9.5 build 20090513 I noticed that it included your original patch even though that patch does not solve the problem.

        Since you closed this tracker already, what do we need to do in order to get the full patch merged in to 1.9.5?

        Show
        Eric Straw added a comment - Dongsheng, When I installed 1.9.5 build 20090513 I noticed that it included your original patch even though that patch does not solve the problem. Since you closed this tracker already, what do we need to do in order to get the full patch merged in to 1.9.5?
        Hide
        Helen Foster added a comment -

        Eric, thanks for your comments and patches. Reopening this issue.

        Show
        Helen Foster added a comment - Eric, thanks for your comments and patches. Reopening this issue.
        Hide
        Helen Foster added a comment -

        Forum discussion on this issue:
        http://moodle.org/mod/forum/discuss.php?d=124128

        Show
        Helen Foster added a comment - Forum discussion on this issue: http://moodle.org/mod/forum/discuss.php?d=124128
        Hide
        Dongsheng Cai added a comment -

        Sorry for delay, Eric, thanks for keep working on this issue, I committed your patch today, let me know if you updated your patch

        BTW, because all moodle scripts use standard Unix text format, soit is better to use unix text format patch, otherwise patch cannot be applied properly.

        Show
        Dongsheng Cai added a comment - Sorry for delay, Eric, thanks for keep working on this issue, I committed your patch today, let me know if you updated your patch BTW, because all moodle scripts use standard Unix text format, soit is better to use unix text format patch, otherwise patch cannot be applied properly.
        Hide
        Dongsheng Cai added a comment -

        Resolved this issue, feel free to reopen it if you have any problem, thanks

        Show
        Dongsheng Cai added a comment - Resolved this issue, feel free to reopen it if you have any problem, thanks
        Hide
        Teresa Gibbison added a comment -

        So the highlighted colour has changed from #FFD911 (orange) to two different blues??? Was there a reason for the colour change? I can admittedly alter my own theme but wondered why the change?

        Show
        Teresa Gibbison added a comment - So the highlighted colour has changed from #FFD911 (orange) to two different blues??? Was there a reason for the colour change? I can admittedly alter my own theme but wondered why the change?
        Hide
        Teresa Gibbison added a comment -

        sorry make that #FFD991 (orange!)

        Show
        Teresa Gibbison added a comment - sorry make that #FFD991 (orange!)
        Hide
        Eric Straw added a comment -

        Teresa...I changed the color when I was working on finding a solution to the bug. I needed something different than the orange to test what was working and what was not working. I actually used about six different colors. After solving the highlighting problem I figured I could not submit a proposed patch with multiple odd colors. However, I also felt it was important to demonstrate that this patch solved the problem. The first patch submitted above did not solve the problem but you could not easily tell that because the patch stuck with the orange highlighting. Thus, I wanted something different than the orange. I chose the blues as the best of the new colors I was working with that were easily seen in all of the standard themes, whether highlighting or border. My assumption was that after the patch had been reviewed someone responsible for Moodle themes would decide on the correct color to use. I do prefer the blue over the orange, but I never thought the patch would be applied as-is.

        Show
        Eric Straw added a comment - Teresa...I changed the color when I was working on finding a solution to the bug. I needed something different than the orange to test what was working and what was not working. I actually used about six different colors. After solving the highlighting problem I figured I could not submit a proposed patch with multiple odd colors. However, I also felt it was important to demonstrate that this patch solved the problem. The first patch submitted above did not solve the problem but you could not easily tell that because the patch stuck with the orange highlighting. Thus, I wanted something different than the orange. I chose the blues as the best of the new colors I was working with that were easily seen in all of the standard themes, whether highlighting or border. My assumption was that after the patch had been reviewed someone responsible for Moodle themes would decide on the correct color to use. I do prefer the blue over the orange, but I never thought the patch would be applied as-is.

          People

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

            Dates

            • Created:
              Updated:
              Resolved: