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
    • Rank:
      41843

      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.

        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: