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

Fix up the CSS used to display dimmed objects.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0.8, 2.1.5, 2.2.2, 2.3
    • Fix Version/s: 2.0.9, 2.1.6, 2.2.3
    • Component/s: Themes, Usability
    • Labels:
    • Testing Instructions:
      Hide

      note: CSS knowledge needed for test.

      1) Go to the courses weekly outline.
      2) Add a resource - label.
      3) Enter in Label text. Set visible to hide.
      4) Check to make sure that text is grey.
      5) Edit the label and change the text colour to red.
      6) save and return to course - ensure that the label is now dimmed.

      Please test in IE as well as other browsers.

      A) add custom css to your theme using 'dimmed_category' and check its not dimmed but is actually grey (#AAA) when using 'dimmed_category' on 'a' (Anchors)

      Show
      note: CSS knowledge needed for test. 1) Go to the courses weekly outline. 2) Add a resource - label. 3) Enter in Label text. Set visible to hide. 4) Check to make sure that text is grey. 5) Edit the label and change the text colour to red. 6) save and return to course - ensure that the label is now dimmed. Please test in IE as well as other browsers. A) add custom css to your theme using 'dimmed_category' and check its not dimmed but is actually grey (#AAA) when using 'dimmed_category' on 'a' (Anchors)
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      As part of MDL-26712 the styles for dimmed objects including categories, labels etc were changed from being a grey colour to being 50% transparent.
      This was a smart solution to the issue with labels that was being addressed however it has ramifications in that those dimmed classes are used all over the place by both ourselves (core) and others.
      There's no telling where they were used and the new styles, particularly the height:1% are liable to cause problems.
      As well as this the solution implemented is not backwards compatible, if third party theme's have overridden dimmed font colour they will have been left with a faded version of the colour they used.

      On top of that is that we avoid adding these sort of styles to the base theme unless absolutely 100% necessary.
      The main reasons for this being:

      1. The design of base - think of it like white bread - it is meant to be kept plain and simple, nothing exciting happens there unless it is 100% required.
      2. Cross browser comparability and the rule complexities used to solve it often lead to browser variations and problems for third party dev's in overriding the rules. This in part relates to the design of the base theme, we keep things simple so that it is easy to override. Remember customising a theme is the first thing people look to do in establishing a site.
      3. Usability issues, in regards to opacity especially in area's containing user content when background colours and font colours may not be clear with diminished opacity.

      In regards to these changes as mentioned earlier I think the solution implemented is a good solution for hidden labels. The problem is really that rather than implement it to target hidden labels specifically, the general css for dimming was changed.
      The introduction of the 1% height also needs to be reviewed... certainly at the moment that is the single biggest problem.

        Gliffy Diagrams

          Issue Links

            Activity

            samhemelryk Sam Hemelryk created issue -
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Bumping the priority on this, theme changes like this to the base theme need to be minimised as nearly all theme's out there extend it and the chances of people reacting to this change by modifying there own code are pretty high.

            Show
            samhemelryk Sam Hemelryk added a comment - Bumping the priority on this, theme changes like this to the base theme need to be minimised as nearly all theme's out there extend it and the chances of people reacting to this change by modifying there own code are pretty high.
            samhemelryk Sam Hemelryk made changes -
            Field Original Value New Value
            Priority Minor [ 4 ] Major [ 3 ]
            salvetore Michael de Raadt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Labels triaged
            Hide
            adamann2 Ann Adamcik added a comment -

            This is causing an issue in the gradebook. For hidden activities on the 'Categories and items' page, the opacity setting gets applied twice - once for the dimmed row and once again for the link. In the attached screenshot, you can see that the hidden assignment is much lighter than the hidden manual grade item. Hidden activities are nearly unreadable, and are not consistent with hidden manual grade items.

            Show
            adamann2 Ann Adamcik added a comment - This is causing an issue in the gradebook. For hidden activities on the 'Categories and items' page, the opacity setting gets applied twice - once for the dimmed row and once again for the link. In the attached screenshot, you can see that the hidden assignment is much lighter than the hidden manual grade item. Hidden activities are nearly unreadable, and are not consistent with hidden manual grade items.
            adamann2 Ann Adamcik made changes -
            Attachment screenshot-1.jpg [ 26278 ]
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks for identifying the regression Ann, I'm increasing the priority on this issue to get it onto our development que a little faster.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks for identifying the regression Ann, I'm increasing the priority on this issue to get it onto our development que a little faster.
            samhemelryk Sam Hemelryk made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            bostelm Henning Bostelmann made changes -
            Link This issue has been marked as being related by MDL-30524 [ MDL-30524 ]
            lazydaisy Mary Evans made changes -
            Assignee Patrick Malley [ ptrkmkl ] Mary Evans [ lazydaisy ]
            lazydaisy Mary Evans made changes -
            Link This issue duplicates MDL-30524 [ MDL-30524 ]
            lazydaisy Mary Evans made changes -
            Link This issue duplicates MDL-30524 [ MDL-30524 ]
            lazydaisy Mary Evans made changes -
            Status Open [ 1 ] Development in progress [ 3 ]
            Hide
            lazydaisy Mary Evans added a comment -

            @Sam,

            What do I do with this...put it back to what it was?
            Or do you have another suggestion?

            Show
            lazydaisy Mary Evans added a comment - @Sam, What do I do with this...put it back to what it was? Or do you have another suggestion?
            lazydaisy Mary Evans made changes -
            Status Development in progress [ 3 ] Peer review in progress [ 10013 ]
            Peer reviewer samhemelryk
            lazydaisy Mary Evans made changes -
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Hi Mary,

            Setting it back to how it was is probably the best solution, I'm betting there are a lot of theme's out there with really dulled out dimmed items because of this change.
            However it should probably be investigated. Have you changed any of your dimmed settings etc? and have you noticed any problems?

            I think the minimum is to remove the 1% height, if the opacity is working for people then cool that could stay, but the 1% height is a problem.

            Also as part of this you should have a quick look at the original problem MDL-26712 and just check that the new solution still works there, and if not create a quick fix for it.

            Cheers
            Sam

            Show
            samhemelryk Sam Hemelryk added a comment - Hi Mary, Setting it back to how it was is probably the best solution, I'm betting there are a lot of theme's out there with really dulled out dimmed items because of this change. However it should probably be investigated. Have you changed any of your dimmed settings etc? and have you noticed any problems? I think the minimum is to remove the 1% height, if the opacity is working for people then cool that could stay, but the 1% height is a problem. Also as part of this you should have a quick look at the original problem MDL-26712 and just check that the new solution still works there, and if not create a quick fix for it. Cheers Sam
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Looks good now thanks Mary feel free to create branches for other versions as required and get it up for integration

            Show
            samhemelryk Sam Hemelryk added a comment - Looks good now thanks Mary feel free to create branches for other versions as required and get it up for integration
            samhemelryk Sam Hemelryk made changes -
            Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
            lazydaisy Mary Evans made changes -
            Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
            Hide
            lazydaisy Mary Evans added a comment -

            All branches done!

            Show
            lazydaisy Mary Evans added a comment - All branches done!
            lazydaisy Mary Evans made changes -
            Link This issue will help resolve MDL-30524 [ MDL-30524 ]
            lazydaisy Mary Evans made changes -
            Link This issue has been marked as being related by MDL-30524 [ MDL-30524 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Some hours ago...

            the main moodle.git repository has 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
            stronk7 Eloy Lafuente (stronk7) added a comment - Some hours ago... the main moodle.git repository has 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
            lazydaisy Mary Evans added a comment -

            REBASED!

            Show
            lazydaisy Mary Evans added a comment - REBASED!
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            nebgor Aparup Banerjee made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator nebgor
            Hide
            nebgor Aparup Banerjee added a comment -

            Hi all,

            I've fixed up a code formating for the 21 branch and also tweaked the commit message to show the MDL-xxxxx instead of the git branch (MDL-xxxxx_yy_base).

            Thanks for this fix, its been integrated into master, 22, 21 and as far as it was a regression from a fix made to 20, its also been backported to 20.

            ready for testing!

            Tester: testing instructions are replicated from MDL-26712

            • also add custom css to your theme and check its not dimmed but is grey when using 'dimmed_category'
            Show
            nebgor Aparup Banerjee added a comment - Hi all, I've fixed up a code formating for the 21 branch and also tweaked the commit message to show the MDL-xxxxx instead of the git branch (MDL-xxxxx_yy_base). Thanks for this fix, its been integrated into master, 22, 21 and as far as it was a regression from a fix made to 20, its also been backported to 20. ready for testing! Tester: testing instructions are replicated from MDL-26712 also add custom css to your theme and check its not dimmed but is grey when using 'dimmed_category'
            nebgor Aparup Banerjee made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Affects Version/s 2.2.2 [ 11552 ]
            Affects Version/s 2.1.5 [ 11553 ]
            Affects Version/s 2.0.8 [ 11554 ]
            Affects Version/s 2.3 [ 10657 ]
            Affects Version/s 2.2 [ 10656 ]
            Fix Version/s 2.0.9 [ 12051 ]
            Fix Version/s 2.1.6 [ 12052 ]
            Fix Version/s 2.2.3 [ 12053 ]
            Fix Version/s STABLE backlog [ 10463 ]
            nebgor Aparup Banerjee made changes -
            Testing Instructions note: CSS knowledge needed for test.

            1) Go to the courses weekly outline.
            2) Add a resource - label.
            3) Enter in Label text. Set visible to hide.
            4) Check to make sure that text is grey.
            5) Edit the label and change the text colour to red.
            6) save and return to course - ensure that the label is now dimmed.

            Please test in IE as well as other browsers.

            A) add custom css to your theme using 'dimmed'category' and check its not dimmed but is actually grey (#AAA) when using 'dimmed_category' on 'a' (Anchors)

            Hide
            nebgor Aparup Banerjee added a comment -

            ah, it skipped my eye initially but i've added a commit to fix typo ']' --> '}' in master branch.

            Show
            nebgor Aparup Banerjee added a comment - ah, it skipped my eye initially but i've added a commit to fix typo ']' --> '}' in master branch.
            Hide
            nebgor Aparup Banerjee added a comment -

            Mary, you may want to check out the 'git cherry-pick' command. this will save you from copying and pasting code between branches. instead it will attempt to pick the commit with the changes and apply it exactly to another branch.

            Show
            nebgor Aparup Banerjee added a comment - Mary, you may want to check out the 'git cherry-pick' command. this will save you from copying and pasting code between branches. instead it will attempt to pick the commit with the changes and apply it exactly to another branch.
            Hide
            lazydaisy Mary Evans added a comment - - edited

            Hi Aparup

            I do use cherry-pick' a lot usually, I skiped using it for this as I did not want to run into problems, which I have done in the past, but looks like I created some on the way, reading between the lines at your last couple of comments!

            Did I put [ instead of {

            Also what did I do wrong in the commit message?

            Cheers
            Mary

            Show
            lazydaisy Mary Evans added a comment - - edited Hi Aparup I do use cherry-pick' a lot usually, I skiped using it for this as I did not want to run into problems, which I have done in the past, but looks like I created some on the way, reading between the lines at your last couple of comments! Did I put [ instead of { Also what did I do wrong in the commit message? Cheers Mary
            lazydaisy Mary Evans made changes -
            Testing Instructions note: CSS knowledge needed for test.

            1) Go to the courses weekly outline.
            2) Add a resource - label.
            3) Enter in Label text. Set visible to hide.
            4) Check to make sure that text is grey.
            5) Edit the label and change the text colour to red.
            6) save and return to course - ensure that the label is now dimmed.

            Please test in IE as well as other browsers.

            A) add custom css to your theme using 'dimmed'category' and check its not dimmed but is actually grey (#AAA) when using 'dimmed_category' on 'a' (Anchors)

            note: CSS knowledge needed for test.

            1) Go to the courses weekly outline.
            2) Add a resource - label.
            3) Enter in Label text. Set visible to hide.
            4) Check to make sure that text is grey.
            5) Edit the label and change the text colour to red.
            6) save and return to course - ensure that the label is now dimmed.

            Please test in IE as well as other browsers.

            A) add custom css to your theme using 'dimmed_category' and check its not dimmed but is actually grey (#AAA) when using 'dimmed_category' on 'a' (Anchors)

            salvetore Michael de Raadt made changes -
            Tester salvetore
            Hide
            nebgor Aparup Banerjee added a comment -

            no worries there Mary

            cherry-pick is fantastic! Heh i've been happy to handle cherry-pick's problems - since it saves lot of other problems :-D - ( yea '[' instead of '{' but fixed! )

            the commit message had your branch name before the colon "MDL-31008 theme_base: reversed MDL-26712 by making dimmed objects gre…"
            it just needs to be the component, so i just tweaked it to " MDL-31008 themes : (base theme) reversed MDL-26712 by making dimmed objects grey an..."

            Thanks for your efforts Mary!

            Show
            nebgor Aparup Banerjee added a comment - no worries there Mary cherry-pick is fantastic! Heh i've been happy to handle cherry-pick's problems - since it saves lot of other problems :-D - ( yea '[' instead of '{' but fixed! ) the commit message had your branch name before the colon " MDL-31008 theme_base: reversed MDL-26712 by making dimmed objects gre…" it just needs to be the component, so i just tweaked it to " MDL-31008 themes : (base theme) reversed MDL-26712 by making dimmed objects grey an..." Thanks for your efforts Mary!
            salvetore Michael de Raadt made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            salvetore Michael de Raadt added a comment -

            Test result: Success.

            I tested labels with colour, links and forcing the dimmed_category class. Tested across 2.1, 2.2 and master in FF, IE, Chrome and Opera.

            Show
            salvetore Michael de Raadt added a comment - Test result: Success. I tested labels with colour, links and forcing the dimmed_category class. Tested across 2.1, 2.2 and master in FF, IE, Chrome and Opera.
            salvetore Michael de Raadt made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            FCT (fixed, closing, thanks). Ciao

            "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!"
            ~ Benjamin Disraeli

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - FCT (fixed, closing, thanks). Ciao "I feel a very unusual sensation - if it is not indigestion, I think it must be gratitude!" ~ Benjamin Disraeli
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 15/Mar/12
            Subversion JIRA

            Links Hierarchy

             Documentation

            Invalid license: EXPIRED

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  14/May/12