Uploaded image for project: '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
    • Status: Closed
    • Priority: 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

          Attachments

            Activity

            Hide
            lazydaisy 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
            lazydaisy 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
            daniss 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
            daniss 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
            lazydaisy 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
            lazydaisy 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
            daniss 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
            daniss 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
            poltawski 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
            poltawski 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
            rwijaya Rossiani Wijaya added a comment -

            Testing this in 2.4 only.

            It works as expected.

            Test passed.

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

            Closing as fixed, many thanks for your awesome collaboration.

            Show
            stronk7 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:
                  Fix Release Date:
                  3/Dec/12