Moodle
  1. Moodle
  2. MDL-36960

Block titles are not visible when blocks are docked

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.4
    • Fix Version/s: 2.4.1
    • Component/s: Blocks, Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Add the following to the bottom of theme/base/style/core.css: *h2 {line-height:40px;}

        *

      2. Browse to your site and dock a block.
      3. Make sure that the title of the block in the dock is shown correctly.
      Show
      Add the following to the bottom of theme/base/style/core.css: *h2 {line-height:40px;} * Browse to your site and dock a block. Make sure that the title of the block in the dock is shown correctly.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE
    • Pull Master Branch:
      wip-MDL-36960-m24
    • Rank:
      46500

      Description

      This particular theme was working OK in 2.3.3. but testing it this weekend I found that when blocks are docked the titles are displayed outside the dock and therefore not visible, being white on white. The theme in question (Tiny) is a custom Moodle theme using a grid type page-layout based on Twitter Bootstrap.

      You can see the problem in the attached image.

        Activity

        Hide
        Mary Evans added a comment - - edited

        You can clearly see the location of the docked title, highlighted in red, and the accompanying inline styles that are added, I presume, via dock.js.

        Show
        Mary Evans added a comment - - edited You can clearly see the location of the docked title, highlighted in red, and the accompanying inline styles that are added, I presume, via dock.js.
        Hide
        Mary Evans added a comment -

        I tested this again just now and get a totally different setting this time 7.4px but this still shows it in the same position as the image above. It actually needs it to be right: 35px to get it into place, but then it is not in the center but moved up towards the top of the tab.

        Show
        Mary Evans added a comment - I tested this again just now and get a totally different setting this time 7.4px but this still shows it in the same position as the image above. It actually needs it to be right: 35px to get it into place, but then it is not in the center but moved up towards the top of the tab.
        Hide
        Mary Evans added a comment -

        Sorry to push this on you Sam, but I need to know why this is happening and how to fix it.

        Show
        Mary Evans added a comment - Sorry to push this on you Sam, but I need to know why this is happening and how to fix it.
        Hide
        Mary Evans added a comment -

        @Amy
        Adding you as a watcher to keep you in the loop.

        Show
        Mary Evans added a comment - @Amy Adding you as a watcher to keep you in the loop.
        Hide
        Amy Groshek added a comment -

        @Mary thanks for including me. I am able to reproduce.

        Have located the place where the positioning is set in M.core_dock.fixTitleOrientation. Beginning at line 516:

        // We need to fix a font-size - sorry theme designers.
            var fontsize = '11px';
            var transform = (clockwise) ? 'rotate(90deg)' : 'rotate(270deg)';
            var test = Y.Node.create('<h2><span style="font-size:'+fontsize+';position:absolute;">'+text+'</span></h2>');
            this.nodes.body.insert(test, 0);
            var width = test.one('span').get('offsetWidth') * 1.2;
            var height = test.one('span').get('offsetHeight');
            test.remove();
        
            title.setContent(text);
            title.addClass('css3transform');
        
            // Move the title into position
            title.setStyles({
                'margin' : '0',
                'padding' : '0',
                'position' : 'relative',
                'fontSize' : fontsize,
                'width' : width,
                'top' : width/2
            });
        
            // Positioning is different when in RTL mode.
            if (right_to_left()) {
                title.setStyle('left', width/2 - height);
            } else {
                title.setStyle('right', width/2 - height);
            }
        
        

        I did get a chuckle out of the apology comment.

        I am able to trace out the width and height values gotten from the test object. Their dimensions are pretty much the same in standard theme and tiny theme, but somehow the calculations for right positioning do not come out right for the titles in the tiny theme dock, and they end up with a negative value for right.

        I did try one thing, and that is eliminating the <span> tag from inside the <h2> tag (in test object). I did this thinking that the end-product is just an <h2> and the measurement might be more precise. When I did this, the first title was actually positioned correctly in the dock, but following titles were not.

        With the rotation I think I would have to diagram it all out to fully understand what values are doing what. For example, where height is actually height and where it is the width of the rotated object, etc.

        Show
        Amy Groshek added a comment - @Mary thanks for including me. I am able to reproduce. Have located the place where the positioning is set in M.core_dock.fixTitleOrientation. Beginning at line 516: // We need to fix a font-size - sorry theme designers. var fontsize = '11px'; var transform = (clockwise) ? 'rotate(90deg)' : 'rotate(270deg)'; var test = Y.Node.create('<h2><span style= "font-size:'+fontsize+';position:absolute;" >'+text+'</span></h2>'); this .nodes.body.insert(test, 0); var width = test.one('span').get('offsetWidth') * 1.2; var height = test.one('span').get('offsetHeight'); test.remove(); title.setContent(text); title.addClass('css3transform'); // Move the title into position title.setStyles({ 'margin' : '0', 'padding' : '0', 'position' : 'relative', 'fontSize' : fontsize, 'width' : width, 'top' : width/2 }); // Positioning is different when in RTL mode. if (right_to_left()) { title.setStyle('left', width/2 - height); } else { title.setStyle('right', width/2 - height); } I did get a chuckle out of the apology comment. I am able to trace out the width and height values gotten from the test object. Their dimensions are pretty much the same in standard theme and tiny theme, but somehow the calculations for right positioning do not come out right for the titles in the tiny theme dock, and they end up with a negative value for right. I did try one thing, and that is eliminating the <span> tag from inside the <h2> tag (in test object). I did this thinking that the end-product is just an <h2> and the measurement might be more precise. When I did this, the first title was actually positioned correctly in the dock, but following titles were not. With the rotation I think I would have to diagram it all out to fully understand what values are doing what. For example, where height is actually height and where it is the width of the rotated object, etc.
        Hide
        Mary Evans added a comment -

        Hi Amy,

        Me too with the apology!

        I was comparing this code with Moodle 2.3.3 but was not able to understand why they needed to change it, as the dock titles work OK in 2.3.3.

        Where are 'offsetHeight' and 'offsetWidth' coming from?

        Show
        Mary Evans added a comment - Hi Amy, Me too with the apology! I was comparing this code with Moodle 2.3.3 but was not able to understand why they needed to change it, as the dock titles work OK in 2.3.3. Where are 'offsetHeight' and 'offsetWidth' coming from?
        Hide
        Amy Groshek added a comment - - edited

        I haven't compared to M2.3 yet. Good idea. Those are YUI properties. A fake element containing the title is injected into the page, then the width and height of that fake element are gotten (with some alterations). Then the fake element is removed. Subsequent calculations for the positioning of the actual title are made using the gotten values.

        offsetWidth is a DOM property meaning, roughly, "the full width of this element including padding and border."
        http://help.dottoro.com/ljfwvsrv.php

        M23, if I remember right, was using the SVG <text> element. It had some issues of its own. I definitely favor this approach.

        Show
        Amy Groshek added a comment - - edited I haven't compared to M2.3 yet. Good idea. Those are YUI properties. A fake element containing the title is injected into the page, then the width and height of that fake element are gotten (with some alterations). Then the fake element is removed. Subsequent calculations for the positioning of the actual title are made using the gotten values. offsetWidth is a DOM property meaning, roughly, "the full width of this element including padding and border." http://help.dottoro.com/ljfwvsrv.php M23, if I remember right, was using the SVG <text> element. It had some issues of its own. I definitely favor this approach.
        Hide
        Sam Hemelryk added a comment -

        Hi ladies,

        Things were changed for 2.4 from using SVG to rotate the text to using CSS3 to rotate the text.
        SVG had a few problems, the anti-aliasing of the text in certain browsers was a bit dodgy, and creating SVG using JS in general isn't a great approach.
        At the time I wrote the doc SVG was better supported than CSS3, however times have changed CSS3 is better supported and its definitely the better approach.

        The offsetWidth and offsetHeight properties are just general JS properties that all elements support btw, nothing special to YUI.
        They're a little bit hard to work with sometimes as there's so much that can affect the height and width of a element.

        I tracked down the tiny theme and installed it to have a look at what was going wrong.
        The problem appears to the be line height.
        Looking at the test element I see that it has a fixed line height of 40px, where as once displayed in the dock it has a line height of 100%.
        That change is effecting the height that is read from the test element.
        The solution is to patch the dock.
        At present there is no easy way to work around this (you could change the fixed line-height but thats not a good solution).
        The dock needs to be patched, three things need to happen.

        1. Add a specific class to the test element that gets created so that we can target it with CSS.
        2. Move the inline CSS into theme/base/style/dock.css
        3. Add a line of CSS: "line-height:normal;"

        Doing that will fix the issue, and mean that we can target the test element within CSS allowing us to better address (and work around) future issues of this nature.

        Does that make sense to the both of you?
        I have to get onto an important bug in 2.4 but will come back and produce a branch shortly.

        Many thanks
        Sam

        Show
        Sam Hemelryk added a comment - Hi ladies, Things were changed for 2.4 from using SVG to rotate the text to using CSS3 to rotate the text. SVG had a few problems, the anti-aliasing of the text in certain browsers was a bit dodgy, and creating SVG using JS in general isn't a great approach. At the time I wrote the doc SVG was better supported than CSS3, however times have changed CSS3 is better supported and its definitely the better approach. The offsetWidth and offsetHeight properties are just general JS properties that all elements support btw, nothing special to YUI. They're a little bit hard to work with sometimes as there's so much that can affect the height and width of a element. I tracked down the tiny theme and installed it to have a look at what was going wrong. The problem appears to the be line height. Looking at the test element I see that it has a fixed line height of 40px, where as once displayed in the dock it has a line height of 100%. That change is effecting the height that is read from the test element. The solution is to patch the dock. At present there is no easy way to work around this (you could change the fixed line-height but thats not a good solution). The dock needs to be patched, three things need to happen. Add a specific class to the test element that gets created so that we can target it with CSS. Move the inline CSS into theme/base/style/dock.css Add a line of CSS: "line-height:normal;" Doing that will fix the issue, and mean that we can target the test element within CSS allowing us to better address (and work around) future issues of this nature. Does that make sense to the both of you? I have to get onto an important bug in 2.4 but will come back and produce a branch shortly. Many thanks Sam
        Hide
        Amy Groshek added a comment - - edited

        Sam, I made the changes you listed and pushed to branch detailed above.

        Show
        Amy Groshek added a comment - - edited Sam, I made the changes you listed and pushed to branch detailed above.
        Hide
        Sam Hemelryk added a comment -

        Looks spot on thanks Amy, I've put this up for integration review now.

        Show
        Sam Hemelryk added a comment - Looks spot on thanks Amy, I've put this up for integration review now.
        Hide
        Mary Evans added a comment -

        @Amy & Sam

        WOW! I am impressed!
        Well done.
        Thanks

        Show
        Mary Evans added a comment - @Amy & Sam WOW! I am impressed! Well done. Thanks
        Hide
        Dan Poltawski added a comment -

        Thanks everyone - i've integrated this now

        Show
        Dan Poltawski added a comment - Thanks everyone - i've integrated this now
        Hide
        Dan Poltawski added a comment -

        (Assigning to Amy to reflect the commit )

        Show
        Dan Poltawski added a comment - (Assigning to Amy to reflect the commit )
        Hide
        Dan Poltawski added a comment -

        Tested and passed

        Show
        Dan Poltawski added a comment - Tested and passed
        Hide
        Amy Groshek added a comment -

        @Dan, you're a sweetheart. All I did was follow instructions.

        Show
        Amy Groshek added a comment - @Dan, you're a sweetheart. All I did was follow instructions.
        Hide
        Mary Evans added a comment -

        @Dan
        You beat me to it. I meant to assign this to Amy.

        @Amy
        I am glad you understand all this...it's way above my head. LOL

        Show
        Mary Evans added a comment - @Dan You beat me to it. I meant to assign this to Amy. @Amy I am glad you understand all this...it's way above my head. LOL
        Hide
        Eloy Lafuente (stronk7) added a comment -

        This has landed upstream, closing, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - This has landed upstream, closing, thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: