Moodle
  1. Moodle
  2. MDL-38052

formal_white header displays badly

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.4.2, 2.5
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. In at least IE9 and one other supported browser
      2. Select Formal White theme using the Theme selector and NOT by URL (this is important).
      3. Navigate to the Formal White settings page found at Site Administration > Appearance > Themes > Formal white.
      4. In the Custom Logo & Frontpage Logo settings add the url from the attached greenlandscape.jpg.
        https://tracker.moodle.org/secure/attachment/31338/greenlandscape.jpg
        

        (Save settings).

      5. If not already set, enable Theme Designer Mode in Theme settings. (Save settings).
      6. TEST to see that there is no gap between the page-header and the outer frame other than the white border that is there normally and that the logo sits nicely within it.
      7. Open a new browser tab and navigate back to Formal White settings page.
      8. Set 'Frame margin' to 50px. (Save settings).
      9. Go back to first browser tab and TEST that the login/logout and lang menu are still in the page header, relative to their surroundings.

      TEST in RTL language

      1. With the above settings in place, and everything looking correct, swap to a RTL language.
      2. TEST to see that the Login/logout (headermenu) has shifted to the left side and that the logo is right aligned. You will see this better with a screen resolution 1024px x 786px, or by floating the browser window and making it narrower.
      Show
      In at least IE9 and one other supported browser Select Formal White theme using the Theme selector and NOT by URL (this is important). Navigate to the Formal White settings page found at Site Administration > Appearance > Themes > Formal white. In the Custom Logo & Frontpage Logo settings add the url from the attached greenlandscape.jpg. https: //tracker.moodle.org/secure/attachment/31338/greenlandscape.jpg (Save settings). If not already set, enable Theme Designer Mode in Theme settings. (Save settings). TEST to see that there is no gap between the page-header and the outer frame other than the white border that is there normally and that the logo sits nicely within it. Open a new browser tab and navigate back to Formal White settings page. Set 'Frame margin' to 50px. (Save settings). Go back to first browser tab and TEST that the login/logout and lang menu are still in the page header, relative to their surroundings. TEST in RTL language With the above settings in place, and everything looking correct, swap to a RTL language. TEST to see that the Login/logout (headermenu) has shifted to the left side and that the logo is right aligned. You will see this better with a screen resolution 1024px x 786px, or by floating the browser window and making it narrower.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-38052_M24
    • Pull Master Branch:
      MDL-38052_master
    • Rank:
      47848

      Description

      I recently found the header of formal_white no longer display as it was few months ago.
      Looking at the history of the theme I found that MDL-32806 seems to be origin of the problem.
      In spite of this, I see that the peer reviewer of that issue was me even if I have no memory at all of it.
      Anyway, the problem is well described by the attached two screen shots.
      The first shows the problem, the second the expected solution.
      What I really do not know is whether this fix goes to modify the RTL as improved by MDL-32806.

      1. expected solution.png
        261 kB
      2. greenlandscape.jpg
        197 kB
      3. problem.png
        253 kB

        Activity

        Hide
        Mary Evans added a comment -

        Daniel, the image you attached are of a customised header. This is NOT the Logo that this theme is styling. One would suspect that if a custom header image is added then the custom CSS would be added via the Custom CSS box.

        If you are going to make changes to this CORE theme, then I suggest you use the CORE theme Logo to describe the problem and NOT a customised header.

        Show
        Mary Evans added a comment - Daniel, the image you attached are of a customised header. This is NOT the Logo that this theme is styling. One would suspect that if a custom header image is added then the custom CSS would be added via the Custom CSS box. If you are going to make changes to this CORE theme, then I suggest you use the CORE theme Logo to describe the problem and NOT a customised header.
        Hide
        Mary Evans added a comment - - edited

        Daniele my last comment was describing the changes made in MDL-32821 so that the Logo changed sides. If you use the current CORE theme Logo and CSS and NOT a custom background/logo then you will see the login works correctly in relation to the size of the logo and the space between the browser window and the frame.

        Also the fact the .headermenu is set to position:relative is so that it sits in the same place relative to its surroundings. Making this position:absolute fixes this element to the page and so if you increase the space between the browser window and the frame, the absolute position forces the headermenu to stay where it was and so over-hangs the frame.

        Show
        Mary Evans added a comment - - edited Daniele my last comment was describing the changes made in MDL-32821 so that the Logo changed sides. If you use the current CORE theme Logo and CSS and NOT a custom background/logo then you will see the login works correctly in relation to the size of the logo and the space between the browser window and the frame. Also the fact the .headermenu is set to position:relative is so that it sits in the same place relative to its surroundings. Making this position:absolute fixes this element to the page and so if you increase the space between the browser window and the frame, the absolute position forces the headermenu to stay where it was and so over-hangs the frame.
        Hide
        Mary Evans added a comment - - edited

        The problem I see while peer reviewing this, is that your site Logo is wider and so is forced down below the headermenu container.

        If I had wanted to add such a wide image in the header, I would have added it as a background image, rather than a Logo.

        Like I said earlier, custom headers need custom CSS.

        I don't agree with your CSS changes in this case.

        Show
        Mary Evans added a comment - - edited The problem I see while peer reviewing this, is that your site Logo is wider and so is forced down below the headermenu container. If I had wanted to add such a wide image in the header, I would have added it as a background image, rather than a Logo. Like I said earlier, custom headers need custom CSS. I don't agree with your CSS changes in this case.
        Hide
        Mary Evans added a comment -

        Further to my last comment here is the CSS I meant for the b/ground image in your moodle site...

        #page-header {
            background: url{[[pix:theme|bground]]) repeat-x left top; /** assuming the image 
            is repeatable for wide screen **/
            margin: 0;
            padding: 0;
            width: 100%;
            height: 125px; /** or whatever height the header image is **/
        }
        
        Show
        Mary Evans added a comment - Further to my last comment here is the CSS I meant for the b/ground image in your moodle site... #page-header { background: url{[[pix:theme|bground]]) repeat-x left top; /** assuming the image is repeatable for wide screen **/ margin: 0; padding: 0; width: 100%; height: 125px; /** or whatever height the header image is **/ }
        Hide
        Michael de Raadt added a comment -

        Hi, Mary.

        If you're finished reviewing this and ready to pass it back to Daniel, please click "Finish peer review".

        Show
        Michael de Raadt added a comment - Hi, Mary. If you're finished reviewing this and ready to pass it back to Daniel, please click "Finish peer review".
        Hide
        Mary Evans added a comment -

        Daniele,

        I was rather hoping that you would have answered me by now, but perhaps you are rethinking the problem you face with your own modified version of Formal White.

        If it is that the BUG is more to do with the limitations of the width of the Logo, then you can correct that in the description. However, you could create a new setting to allow a background image set in CSS for the page header. This way an organisation could have a coloured image, stripes or patterned and also a small logo at the same time.

        If you would like, I could do this for you so you can see what I am talking about.

        Show
        Mary Evans added a comment - Daniele, I was rather hoping that you would have answered me by now, but perhaps you are rethinking the problem you face with your own modified version of Formal White. If it is that the BUG is more to do with the limitations of the width of the Logo, then you can correct that in the description. However, you could create a new setting to allow a background image set in CSS for the page header. This way an organisation could have a coloured image, stripes or patterned and also a small logo at the same time. If you would like, I could do this for you so you can see what I am talking about.
        Hide
        Daniele Cordella added a comment -

        Ciao all and sorry if I did not reply in time.
        I have some difficulties far from moodle.
        I am quite confused about all your suggestions.
        What I would like to have is a wide picture display correctly as far as the standard moodle logo in RTL such as in LTR.
        I will make more investigation and I will reply.
        Please, do not blame me, if my further answer will require some unexpected extra time.
        Any suggestion is welcome. In the mean time I attach my "green landscape" to help whoever has more goodwill, patience and time than me.

        Show
        Daniele Cordella added a comment - Ciao all and sorry if I did not reply in time. I have some difficulties far from moodle. I am quite confused about all your suggestions. What I would like to have is a wide picture display correctly as far as the standard moodle logo in RTL such as in LTR. I will make more investigation and I will reply. Please, do not blame me, if my further answer will require some unexpected extra time. Any suggestion is welcome. In the mean time I attach my "green landscape" to help whoever has more goodwill, patience and time than me.
        Hide
        Mary Evans added a comment -

        Ciao Daniele,
        I understand...thanks for the image. I will let you know when I have some fixes for this problem for you.

        Show
        Mary Evans added a comment - Ciao Daniele, I understand...thanks for the image. I will let you know when I have some fixes for this problem for you.
        Hide
        Mary Evans added a comment -

        @Daniele I have found a simple way to solve this. Basically all we need to do is add this to the Logo/Frontpage Logo description in theme/formal_white/lang/en/theme_formal_white.php setting $strings...

        $string['customlogourldesc'] = 'Change the logo for this theme
        by entering the full or relatve URL to an image you wish to use
        (i.e. http://www.yoursite.ltd/mylogo.png or ../path/to/your/logo.png).
        As a reference the default logo is 200px wide, 50px high
        and a transparent png will work best.<br>
        If you decide to use a wider logo you may need to adjust the
        top margin so that it displays correctly. To do this try adding
        #headerlogo img {margin-top: -55px;} in the Custom CSS box 
        lower down this page.';
        

        And

        $string['frontpagelogourldesc'] = 'Change the logo that is displayed
        on the front page of your site by entering the full or relatve URL 
        to the image you wish to use (i.e. http://www.yoursite.tld/myfrontpagelogo.png
        or ../path/to/your/logo.png). This setting overrides the custom logo setting.
        As a reference the default logo is 300px wide, 80px high and a transparent png
        will work best.<br>If you decide to use a wider logo you may need to adjust
        the top margin so that it displays correctly. To do this try adding
        #page-site-index #headerlogo img {margin-top: -55px;} in the Custom CSS box
        lower down this page.';
        
        Show
        Mary Evans added a comment - @Daniele I have found a simple way to solve this. Basically all we need to do is add this to the Logo/Frontpage Logo description in theme/formal_white/lang/en/theme_formal_white.php setting $strings... $string['customlogourldesc'] = 'Change the logo for this theme by entering the full or relatve URL to an image you wish to use (i.e. http: //www.yoursite.ltd/mylogo.png or ../path/to/your/logo.png). As a reference the default logo is 200px wide, 50px high and a transparent png will work best.<br> If you decide to use a wider logo you may need to adjust the top margin so that it displays correctly. To do this try adding #headerlogo img {margin-top: -55px;} in the Custom CSS box lower down this page.'; And $string['frontpagelogourldesc'] = 'Change the logo that is displayed on the front page of your site by entering the full or relatve URL to the image you wish to use (i.e. http: //www.yoursite.tld/myfrontpagelogo.png or ../path/to/your/logo.png). This setting overrides the custom logo setting. As a reference the default logo is 300px wide, 80px high and a transparent png will work best.<br>If you decide to use a wider logo you may need to adjust the top margin so that it displays correctly. To do this try adding #page-site-index #headerlogo img {margin-top: -55px;} in the Custom CSS box lower down this page.';
        Hide
        Daniele Cordella added a comment - - edited

        Ciao Mary and thanks for your time and commitment in this issue.
        You are pushing me from my lazy position due to personal problems. Thanks.

        I finally got the peace to understand your comments: "Also the fact the .headermenu is set to position:relative is so that it sits in the same place relative to its surroundings. Making this position:absolute fixes this element to the page and so if you increase the space between the browser window and the frame, the absolute position forces the headermenu to stay where it was and so over-hangs the frame."

        I agree with you, but even this I disagree with your solution.
        As far as I can understand, your solution sounds like: "if you plan to use a not standard logo, adjust the code by your own!" mmmhhhh, in my honest opinion, this is not exciting.

        At the light of your suggestions, I wrote a different solution. I checked the github web site and I found two lines of code I do not recognize as mine. They are:
        /* js help messages */
        #helppopupbox .helpheading

        {margin-top:1em;}

        Probably they come out because my github not not well updated.

        Apart from these, my solution is in: https://github.com/kordan/moodle/compare/MDL-38052_master

        What I got is:
        -> correct position for headermenu with standard and custom theme logo both
        -> correct position for headermenu with RTL and LTR languages
        -> correct position for headermenu for each amount of space between the browser window and the frame
        -> shifted the headermenu to the left for RTL languages
        -> fixed an oooooooold issue I did not even know.

        Please let me know whether you agree with my solution or if a better solution is needed.
        Thanks again, Mary for all your effort.

        Show
        Daniele Cordella added a comment - - edited Ciao Mary and thanks for your time and commitment in this issue. You are pushing me from my lazy position due to personal problems. Thanks. I finally got the peace to understand your comments: "Also the fact the .headermenu is set to position:relative is so that it sits in the same place relative to its surroundings. Making this position:absolute fixes this element to the page and so if you increase the space between the browser window and the frame, the absolute position forces the headermenu to stay where it was and so over-hangs the frame." I agree with you, but even this I disagree with your solution. As far as I can understand, your solution sounds like: "if you plan to use a not standard logo, adjust the code by your own!" mmmhhhh, in my honest opinion, this is not exciting. At the light of your suggestions, I wrote a different solution. I checked the github web site and I found two lines of code I do not recognize as mine. They are: /* js help messages */ #helppopupbox .helpheading {margin-top:1em;} Probably they come out because my github not not well updated. Apart from these, my solution is in: https://github.com/kordan/moodle/compare/MDL-38052_master What I got is: -> correct position for headermenu with standard and custom theme logo both -> correct position for headermenu with RTL and LTR languages -> correct position for headermenu for each amount of space between the browser window and the frame -> shifted the headermenu to the left for RTL languages -> fixed an oooooooold issue I did not even know. Please let me know whether you agree with my solution or if a better solution is needed. Thanks again, Mary for all your effort.
        Hide
        Daniele Cordella added a comment -

        I am sorry Mary,
        I just note that I limited my investigation to the home page, only.
        I try to include other pages too.

        Show
        Daniele Cordella added a comment - I am sorry Mary, I just note that I limited my investigation to the home page, only. I try to include other pages too.
        Hide
        Daniele Cordella added a comment -

        Grrrr, no! It seems I does not forget all "non-home" pages. Ignore my last comment. My brain is not 100% here.

        Show
        Daniele Cordella added a comment - Grrrr, no! It seems I does not forget all "non-home" pages. Ignore my last comment. My brain is not 100% here.
        Hide
        Mary Evans added a comment -

        Reopened this so you can take over Daniele.

        Show
        Mary Evans added a comment - Reopened this so you can take over Daniele.
        Hide
        Mary Evans added a comment -

        I give up.

        Show
        Mary Evans added a comment - I give up.
        Hide
        Mary Evans added a comment - - edited

        Ciao Daniele,
        Mantenere la calma, questo è solo un piccolo problema e non è stato segnalato da altri utenti solo a te stesso.

        Mi dispiace sentire che hanno preoccupazioni di fuori del lavoro.

        questo -> MDL-38052 dovrà aspettare per un po!

        Show
        Mary Evans added a comment - - edited Ciao Daniele, Mantenere la calma, questo è solo un piccolo problema e non è stato segnalato da altri utenti solo a te stesso. Mi dispiace sentire che hanno preoccupazioni di fuori del lavoro. questo -> MDL-38052 dovrà aspettare per un po!
        Hide
        Mary Evans added a comment -

        This looks OK Daniele.
        Please feel free to submit it for Integration Review when you are ready.

        Thank you

        Show
        Mary Evans added a comment - This looks OK Daniele. Please feel free to submit it for Integration Review when you are ready. Thank you
        Hide
        Daniele Cordella added a comment - - edited

        Ciao Mary and thanks for your preliminary "peer review".
        As of your suggestion, I am going to set the state of this issue to "Waiting for peer review".

        Show
        Daniele Cordella added a comment - - edited Ciao Mary and thanks for your preliminary "peer review". As of your suggestion, I am going to set the state of this issue to "Waiting for peer review".
        Hide
        Mary Evans added a comment - - edited

        Ciao Daniele,

        Your changes look fine and work well in all situations, however, I don't like how you have written the CSS, it could be simplified like so...

        /* headermenu */
        
        .headermenu { /** CSS shared by all **/
            font-size: 90%;
            line-height: 1.7em;
            position: absolute;
            top: 0;
        }
        
        .dir-ltr .headermenu { /** CSS for left to right **/
            float: right;
            margin: 1.7em [[calculated:headermenumargin]] 0 0;
            right: 0;
            text-align: right;
        }
        
        .dir-rtl .headermenu { /** CSS for right to left **/
            float: left;
            margin: 1.7em 0 0 [[calculated:headermenumargin]];
            left: 0;
            text-align: left;
        }
        
        Show
        Mary Evans added a comment - - edited Ciao Daniele, Your changes look fine and work well in all situations, however, I don't like how you have written the CSS, it could be simplified like so... /* headermenu */ .headermenu { /** CSS shared by all **/ font-size: 90%; line-height: 1.7em; position: absolute; top: 0; } .dir-ltr .headermenu { /** CSS for left to right **/ float : right; margin: 1.7em [[calculated:headermenumargin]] 0 0; right: 0; text-align: right; } .dir-rtl .headermenu { /** CSS for right to left **/ float : left; margin: 1.7em 0 0 [[calculated:headermenumargin]]; left: 0; text-align: left; }
        Hide
        Daniele Cordella added a comment -

        I agree with you, Mary.
        Your solution is better than mine.
        Now I have to go but I will be back in afternoon.
        If I will find not your intervention I will post your solution to my github.
        gtg ciao and thanks a lot!

        Show
        Daniele Cordella added a comment - I agree with you, Mary. Your solution is better than mine. Now I have to go but I will be back in afternoon. If I will find not your intervention I will post your solution to my github. gtg ciao and thanks a lot!
        Hide
        Daniele Cordella added a comment -

        ok Mary. I submitted the new solution as agreed on the basis of your suggestion.
        If it is ok for you, we can submit this issue for integration.
        Let me know.

        Show
        Daniele Cordella added a comment - ok Mary. I submitted the new solution as agreed on the basis of your suggestion. If it is ok for you, we can submit this issue for integration. Let me know.
        Hide
        Mary Evans added a comment -

        Looks OK to me.

        Show
        Mary Evans added a comment - Looks OK to me.
        Hide
        Mary Evans added a comment - - edited

        You just need to fix this in Moodle 2.4 also. Then you can submit for Integration review
        Thanks
        Mary

        Show
        Mary Evans added a comment - - edited You just need to fix this in Moodle 2.4 also. Then you can submit for Integration review Thanks Mary
        Hide
        Daniele Cordella added a comment -

        ok, the branch for MOODLE_24_stable has been added.
        Mary, please, submit for integration

        Show
        Daniele Cordella added a comment - ok, the branch for MOODLE_24_stable has been added. Mary, please, submit for integration
        Hide
        Damyon Wiese added a comment -

        Thanks Daniele and Mary,

        I tested the changes and they seemed to fix the display issue. I also checked that the problem does not occur in 23.

        This has been pushed to 24 and master.

        Show
        Damyon Wiese added a comment - Thanks Daniele and Mary, I tested the changes and they seemed to fix the display issue. I also checked that the problem does not occur in 23. This has been pushed to 24 and master.
        Hide
        Adrian Greeve added a comment -

        Tested on the 2.4 and master integration branches. The picture now takes up all of the header area (though I did have it stop on the right hand side, but I'm guessing that this is due to the resolution that I am running on my machine).
        Switching to RTL langauge didn't have any problems either.
        Test passed.

        Show
        Adrian Greeve added a comment - Tested on the 2.4 and master integration branches. The picture now takes up all of the header area (though I did have it stop on the right hand side, but I'm guessing that this is due to the resolution that I am running on my machine). Switching to RTL langauge didn't have any problems either. Test passed.
        Hide
        Daniele Cordella added a comment -

        Thanks Adrian!

        Show
        Daniele Cordella added a comment - Thanks Adrian!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Because

        A
        MARVELOUS
        A       U
        Z  YOU  P
        I  ARE  E
        N  PPL  R
        G       B
          TNKS! 
        

        Closing, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Because A MARVELOUS A U Z YOU P I ARE E N PPL R G B TNKS! Closing, ciao
        Hide
        Daniele Cordella added a comment -

        Thanks Eloy. As usual, you are great!

        Show
        Daniele Cordella added a comment - Thanks Eloy. As usual, you are great!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: