Moodle
  1. Moodle
  2. MDL-33789

Sometimes the background image of the navigation bar is not tall enough to fill the heigh of the navbar in formal_white

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.3
    • Fix Version/s: 2.4, STABLE backlog
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      go to the formal_white setting page
      Set the font size to 16
      go to the notification page
      look at the navigation bar
      you should see a not nice background repeating vertically

      apply the patch
      go again to the notification page
      all should be ok
      look at the navigation bar

      Show
      go to the formal_white setting page Set the font size to 16 go to the notification page look at the navigation bar you should see a not nice background repeating vertically apply the patch go again to the notification page all should be ok look at the navigation bar
    • Affected Branches:
      MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-33789_master

      Description

      Look at the attachment reporting a screen shot of part of the notification page in formal white. You can get it by setting "theme_formal_white | fontsizereference" to 16 pixel.

        Gliffy Diagrams

          Activity

          Hide
          Mary Evans added a comment - - edited

          Why not just make the navebar a fixed height, no padding or margin and make line-height = 2 like so...

          .navbar {
              background-color: #AB9D80;
              background-image: url([[pix:theme|bg_bread]]);
              background-position: left top;
              background-repeat: repeat-x;
              height: 30px;
              line-height: 2;
              margin: 0;
              padding: 0;
              width: 100%;
          }
          

          See attached image.

          Show
          Mary Evans added a comment - - edited Why not just make the navebar a fixed height, no padding or margin and make line-height = 2 like so... .navbar { background-color: #AB9D80; background-image: url([[pix:theme|bg_bread]]); background-position: left top; background-repeat: repeat-x; height: 30px; line-height: 2; margin: 0; padding: 0; width: 100%; } See attached image.
          Hide
          Daniele Cordella added a comment -

          Ciao Mary and thanks for your suggestion.
          Sorry for the delay in this answer but I am looking after to 6 children having holidays and I am in what my friends call "teach free week".

          As far as I can understand, the

          • background-color is supposed to be useless
          • margin: 0; is already set by default
          • padding: 0; is already set by default
          • background-image is ok
          • background-position: left top; is the default
          • background-repeat: repeat-x; is ok
          • height: 30px; is ok

          If I am not wrong, the right .navbar style becomes:
          .navbar

          { background-image:url([[pix:theme|bg_bread]]); background-repeat:repeat-x; line-height:30px; line-height:2; width: 100%; }

          /* 30 pixel is the height of bg_bread */

          Do you agree?
          Am I missing something?

          Thanks in advance.

          Show
          Daniele Cordella added a comment - Ciao Mary and thanks for your suggestion. Sorry for the delay in this answer but I am looking after to 6 children having holidays and I am in what my friends call "teach free week". As far as I can understand, the background-color is supposed to be useless margin: 0; is already set by default padding: 0; is already set by default background-image is ok background-position: left top; is the default background-repeat: repeat-x; is ok height: 30px; is ok If I am not wrong, the right .navbar style becomes: .navbar { background-image:url([[pix:theme|bg_bread]]); background-repeat:repeat-x; line-height:30px; line-height:2; width: 100%; } /* 30 pixel is the height of bg_bread */ Do you agree? Am I missing something? Thanks in advance.
          Hide
          Mary Evans added a comment -

          In answer to your question here which I missed in June!

          You cannot have line-height twice. either it is 30px or 2. You can have height: 30px; and line-height: 2; the latter meaning the height of the text. I tend to use this rather than height.

          Sorry it took me so long!

          Show
          Mary Evans added a comment - In answer to your question here which I missed in June! You cannot have line-height twice. either it is 30px or 2. You can have height: 30px; and line-height: 2; the latter meaning the height of the text. I tend to use this rather than height. Sorry it took me so long!
          Hide
          Daniele Cordella added a comment -

          Ciao Mary
          and thanks for answering.
          You are right, of course! It was a typo, as I already pushed the fix to github according to your suggestion.

          @Andrea: Do you have time to take care of peer review? [I do not think you may have time in these weeks!]
          If so, do you mind if I reset the reviewer field?

          Thanks all.

          Show
          Daniele Cordella added a comment - Ciao Mary and thanks for answering. You are right, of course! It was a typo, as I already pushed the fix to github according to your suggestion. @Andrea: Do you have time to take care of peer review? [I do not think you may have time in these weeks!] If so, do you mind if I reset the reviewer field? Thanks all.
          Hide
          Dan Poltawski added a comment -

          Seems like this includes some other whitespace chanegs which were not related to the patch.

          But for the itnerests of expediency i've itnegrated this.

          Show
          Dan Poltawski added a comment - Seems like this includes some other whitespace chanegs which were not related to the patch. But for the itnerests of expediency i've itnegrated this.
          Hide
          Rossiani Wijaya added a comment -

          Testing this in 2.4 only.

          It works as expected.

          Test passed.

          Show
          Rossiani Wijaya added a comment - Testing this in 2.4 only. It works as expected. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Closing as fixed, many thanks for your awesome collaboration.

          Show
          Eloy Lafuente (stronk7) added a comment - Closing as fixed, many thanks for your awesome collaboration.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: