Moodle
  1. Moodle
  2. MDL-31008

Fix up the CSS used to display dimmed objects.

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical 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:
    • Rank:
      37412

      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.

        Issue Links

          Activity

          Hide
          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
          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.
          Hide
          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
          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.
          Hide
          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
          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.
          Hide
          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
          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?
          Hide
          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
          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
          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
          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
          Hide
          Mary Evans added a comment -

          All branches done!

          Show
          Mary Evans added a comment - All branches done!
          Hide
          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
          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
          Mary Evans added a comment -

          REBASED!

          Show
          Mary Evans added a comment - REBASED!
          Hide
          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
          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'
          Hide
          Aparup Banerjee added a comment -

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

          Show
          Aparup Banerjee added a comment - ah, it skipped my eye initially but i've added a commit to fix typo ']' --> '}' in master branch.
          Hide
          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
          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
          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
          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
          Hide
          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
          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!
          Hide
          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
          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.
          Hide
          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
          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

            People

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

              Dates

              • Created:
                Updated:
                Resolved: