Moodle
  1. Moodle
  2. MDL-38856 META: Issues around Bootstrapbase and Clean theme
  3. MDL-38893

The caching boxes that are present on all other themes are gone in bootstrap

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5, STABLE Sprint 32 Alpha
    • Component/s: Themes
    • Labels:
      None
    • Rank:
      48989

      Description

      the awesome little boxes David added a few weeks ago are gone. While their contents remains, they are no longer rendered nicely.

        Issue Links

          Activity

          Hide
          David Scotson added a comment - - edited

          Some of these styles made it in, but not all of them, possibly there were changes after the two themes diverged though it's also possible I deleted them when I discovered we were hitting the 4095 selector limit as there was a lot of CSS that didn't seem to be achieving very much. That's also why the debug stuff is in it's own file at the end of the CSS as it's less vital.

          The ones that made it are currently in less/moodle/debug.less which is where the rest should be added.

          It would be good if the red/amber/green background colors used the appropriate Bootstrap variables:

          #page-footer .performanceinfo .cachesused .cache-store-stats.nohits {background-color: @errorBackground;}
          #page-footer .performanceinfo .cachesused .cache-store-stats.lowhits {background-color: @warningBackground;}
          #page-footer .performanceinfo .cachesused .cache-store-stats.hihits {background-color: @successBackground;}
          

          The current CSS is probably overqualified in many places and the following would do the job:

          .cache-store-stats.nohits { background-color: @errorBackground;}
          .cache-store-stats.lowhits { background-color: @warningBackground;}
          .cache-store-stats.hihits { background-color: @successBackground;}
          

          There's also quite a few unnecessary rules, some in the sense of they don't do anything as they're repeating existing styles and a few more in the sense that what they do isn't really required and makes it look a bit ugly e.g.:

          Making the text bold, underlining the header (actually this one doesn't seem present in the latest Base, must have been removed), setting a text-indent, setting a min-height, putting a 1px border round the boxes, making the text color black.

          And on the HTML side pretty much none of the spans should be spans, instead either headers or divs (probably tables and or lists but that's probably overkill).

          So maybe something like this would do:

          .performanceinfo .cachesused .cache-stats-heading,
          .performanceinfo .cachesused .cache-total-stats {
              font-weight: bold;
              font-size: 110%;
              margin-top: 0.3em;
          }
          #page-footer .performanceinfo .cachesused .cache-definition-stats {
              margin: .3em;
              display: inline-block;
              vertical-align: top;
              background-color: @wellBackground;
          }
          .cache-store-stats {
              padding: 0 1.3em;
          }
          .cache-store-stats.nohits {
              background-color: @errorBackground;
          }
          .cache-store-stats.lowhits {
              background-color: @warningBackground;
          }
          .cache-store-stats.hihits {
              background-color: @successBackground;
          }
          
          Show
          David Scotson added a comment - - edited Some of these styles made it in, but not all of them, possibly there were changes after the two themes diverged though it's also possible I deleted them when I discovered we were hitting the 4095 selector limit as there was a lot of CSS that didn't seem to be achieving very much. That's also why the debug stuff is in it's own file at the end of the CSS as it's less vital. The ones that made it are currently in less/moodle/debug.less which is where the rest should be added. It would be good if the red/amber/green background colors used the appropriate Bootstrap variables: #page-footer .performanceinfo .cachesused .cache-store-stats.nohits {background-color: @errorBackground;} #page-footer .performanceinfo .cachesused .cache-store-stats.lowhits {background-color: @warningBackground;} #page-footer .performanceinfo .cachesused .cache-store-stats.hihits {background-color: @successBackground;} The current CSS is probably overqualified in many places and the following would do the job: .cache-store-stats.nohits { background-color: @errorBackground;} .cache-store-stats.lowhits { background-color: @warningBackground;} .cache-store-stats.hihits { background-color: @successBackground;} There's also quite a few unnecessary rules, some in the sense of they don't do anything as they're repeating existing styles and a few more in the sense that what they do isn't really required and makes it look a bit ugly e.g.: Making the text bold, underlining the header (actually this one doesn't seem present in the latest Base, must have been removed), setting a text-indent, setting a min-height, putting a 1px border round the boxes, making the text color black. And on the HTML side pretty much none of the spans should be spans, instead either headers or divs (probably tables and or lists but that's probably overkill). So maybe something like this would do: .performanceinfo .cachesused .cache-stats-heading, .performanceinfo .cachesused .cache-total-stats { font-weight: bold; font-size: 110%; margin-top: 0.3em; } #page-footer .performanceinfo .cachesused .cache-definition-stats { margin: .3em; display: inline-block; vertical-align: top; background-color: @wellBackground; } .cache-store-stats { padding: 0 1.3em; } .cache-store-stats.nohits { background-color: @errorBackground; } .cache-store-stats.lowhits { background-color: @warningBackground; } .cache-store-stats.hihits { background-color: @successBackground; }
          Hide
          Martin Dougiamas added a comment -

          Jason could you please test this and submit for integration if it works?

          Show
          Martin Dougiamas added a comment - Jason could you please test this and submit for integration if it works?
          Hide
          Jason Fowler added a comment -

          Sure, will do that tomorrow, will create a branch on git and put the patches into it, and go through the process of getting it integrated. Thanks David for providing the solution.

          Show
          Jason Fowler added a comment - Sure, will do that tomorrow, will create a branch on git and put the patches into it, and go through the process of getting it integrated. Thanks David for providing the solution.
          Hide
          Martin Dougiamas added a comment -

          Ping

          Show
          Martin Dougiamas added a comment - Ping
          Hide
          David Scotson added a comment -

          I'm not sure if I'm supposed to be peer reviewing this, and I'm still not sure exactly what that process entails but from a general perspective I've checked out Jason's branch and it looks good to me, both the changes to the less file and the output.

          Show
          David Scotson added a comment - I'm not sure if I'm supposed to be peer reviewing this, and I'm still not sure exactly what that process entails but from a general perspective I've checked out Jason's branch and it looks good to me, both the changes to the less file and the output.
          Hide
          Mary Evans added a comment -

          You need to rebase this branch Jason. I got conflicts in style/generated.css
          But other than that it's looking good.

          Show
          Mary Evans added a comment - You need to rebase this branch Jason. I got conflicts in style/generated.css But other than that it's looking good.
          Hide
          Jason Fowler added a comment -

          Thanks Mary, not sure how work on the style/generated.css is going to work, we may have to stop generating it for pull branches. Need to talk to the integrators about that

          Show
          Jason Fowler added a comment - Thanks Mary, not sure how work on the style/generated.css is going to work, we may have to stop generating it for pull branches. Need to talk to the integrators about that
          Hide
          Mary Evans added a comment -

          Jason, are you going to rebase this and submit for integration?

          Show
          Mary Evans added a comment - Jason, are you going to rebase this and submit for integration?
          Hide
          Jason Fowler added a comment -

          yes, I spoke to the integrators just a moment ago. will do it ASAP.

          Show
          Jason Fowler added a comment - yes, I spoke to the integrators just a moment ago. will do it ASAP.
          Hide
          Jason Fowler added a comment -

          Rebased.

          Show
          Jason Fowler added a comment - Rebased.
          Hide
          Jason Fowler added a comment -

          Thanks for the review Mary, pushing for integration now.

          Show
          Jason Fowler added a comment - Thanks for the review Mary, pushing for integration now.
          Hide
          Sam Hemelryk added a comment -

          Thanks guys, definitely worth having that improvement in 2.5 as those who use that info definitely appreciate the improved layout!

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks guys, definitely worth having that improvement in 2.5 as those who use that info definitely appreciate the improved layout! Many thanks Sam
          Hide
          Sam Hemelryk added a comment -

          Tested during integration review and passed

          Show
          Sam Hemelryk added a comment - Tested during integration review and passed
          Hide
          Eloy Lafuente (stronk7) added a comment -

          I feel myself really alone tonight! So was time to push your fixes upstream!

          "Lest we forget. We will remember them."

          Thanks and ciao!

          Show
          Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: