Moodle
  1. Moodle
  2. MDL-42187

Label in mform radio buttons are not well aligned to radio buttons in formal_white

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.6
    • Fix Version/s: 2.6
    • Component/s: Themes

      Description

      Recent changes to themes were misaligning mform labels from relative radio buttons in formal_white.
      See attachments to better understand.

        Gliffy Diagrams

        1. newradiotest.php
          2 kB
          Daniele Cordella
        2. radiotest.php
          1 kB
          Daniele Cordella
        1. expected.png
          13 kB
        2. inBase.png
          13 kB
        3. inCanvas.png
          15 kB
        4. MDL-42187-newradiotest.jpg
          73 kB
        5. problem.png
          14 kB

          Activity

          Hide
          Mary Evans added a comment -

          Ciao Daniele, can you please check that base theme looks OK too, and let me know, as the fix may be better in base theme if it is relevant?

          Thanks
          Mary

          Show
          Mary Evans added a comment - Ciao Daniele, can you please check that base theme looks OK too, and let me know, as the fix may be better in base theme if it is relevant? Thanks Mary
          Hide
          Daniele Cordella added a comment -

          Ciao Mary,
          it seems base and canvas are safe of this issue. Look at screen shots. Ciao.

          Show
          Daniele Cordella added a comment - Ciao Mary, it seems base and canvas are safe of this issue. Look at screen shots. Ciao.
          Hide
          Daniele Cordella added a comment -

          CIao Mary
          I attached the file radio_test.php.
          By moving it to your local moodle root (in the same folder of config.php) and visiting it by calling the page http://localhost/moodle26/radiotest.php you can see any detail you may like.

          Show
          Daniele Cordella added a comment - CIao Mary I attached the file radio_test.php. By moving it to your local moodle root (in the same folder of config.php) and visiting it by calling the page http://localhost/moodle26/radiotest.php you can see any detail you may like.
          Hide
          Daniele Cordella added a comment -

          Mary, may I ask you to review this fix with the file I uploaded?
          Thanks in advance.

          Show
          Daniele Cordella added a comment - Mary, may I ask you to review this fix with the file I uploaded? Thanks in advance.
          Hide
          Mary Evans added a comment - - edited

          Ciao Daniele, your patch works OK, however without your patch, if you remove the line-height in your original code at Line 152, the outcome looks like Base theme.
           
          /* needed to vertically align label of mform groups containing, for instance, select and text */
          .mform .fitem

          {    -line-height:2.0em;-    margin-top:0; }

          EDIT:
          Alternatively, if make line-height 1.6 only (no em) it looks as you want it to...

          /* needed to vertically align label of mform groups containing, for instance, select and text */
          .mform .fitem {
              line-height: 1.6;
              margin-top: 0;
          }
          

          Show
          Mary Evans added a comment - - edited Ciao Daniele, your patch works OK, however without your patch, if you remove the line-height in your original code at Line 152, the outcome looks like Base theme.   /* needed to vertically align label of mform groups containing, for instance, select and text */ .mform .fitem {    -line-height:2.0em;-    margin-top:0; } EDIT: Alternatively, if make line-height 1.6 only (no em) it looks as you want it to... /* needed to vertically align label of mform groups containing, for instance, select and text */ .mform .fitem { line-height: 1.6; margin-top: 0; }
          Hide
          Daniele Cordella added a comment -

          following your suggestion and on the basis of my comment to the style selector, I add a select and a text to radiotest.php in order to make a deeper test.

          Show
          Daniele Cordella added a comment - following your suggestion and on the basis of my comment to the style selector, I add a select and a text to radiotest.php in order to make a deeper test.
          Hide
          Daniele Cordella added a comment -

          using newradiotest.php I found something I can not fix making fine tuning through line-height and vertical-align. Can you find some solution?

          Show
          Daniele Cordella added a comment - using newradiotest.php I found something I can not fix making fine tuning through line-height and vertical-align. Can you find some solution?
          Hide
          Daniele Cordella added a comment -

          do not forget to visit the new URL: http://localhost/moodle26/newradiotest.php

          Show
          Daniele Cordella added a comment - do not forget to visit the new URL: http://localhost/moodle26/newradiotest.php
          Hide
          Mary Evans added a comment -

          Try it without vertical-align: baseline; just change the line height to 1.6. Perhaps add margin-bottom to existing code like so...

          .mform .fitem .fitemtitle label

          {    *margin-bottom: 5px;*    padding-right: 5px; }
          Show
          Mary Evans added a comment - Try it without vertical-align: baseline; just change the line height to 1.6. Perhaps add margin-bottom to existing code like so... .mform .fitem .fitemtitle label {    *margin-bottom: 5px;*    padding-right: 5px; }
          Hide
          Mary Evans added a comment -

          See image I just uploaded.

          Show
          Mary Evans added a comment - See image I just uploaded.
          Hide
          Daniele Cordella added a comment - - edited

          Thanks Mary but it still does not work.
          Try to zoom in.
          No... now it works.
          bleak!!! I have to think of it just a bit more.

          Show
          Daniele Cordella added a comment - - edited Thanks Mary but it still does not work. Try to zoom in. No... now it works. bleak!!! I have to think of it just a bit more.
          Hide
          Mary Evans added a comment -

          Zoom is not the best thing to use as this distorts the font. so to stop the distortion, try adding font-size: 108%; and line-height: 1.4;

          I'm just going to test this idea.

          Show
          Mary Evans added a comment - Zoom is not the best thing to use as this distorts the font. so to stop the distortion, try adding font-size: 108%; and line-height: 1.4; I'm just going to test this idea.
          Hide
          Mary Evans added a comment -

          /* needed to vertically align label of mform groups containing, for instance, select and text */
          .mform .fitem

          { line-height: 1.4; font-size: 108%; }
          Show
          Mary Evans added a comment - /* needed to vertically align label of mform groups containing, for instance, select and text */ .mform .fitem { line-height: 1.4; font-size: 108%; }
          Hide
          Daniele Cordella added a comment -

          Thanks Mary!
          I really like your solution that seems to fix each issue.
          I updated my github. If you think my work is well done, please submit for integration this issue.
          Ciao and thanks again.

          Show
          Daniele Cordella added a comment - Thanks Mary! I really like your solution that seems to fix each issue. I updated my github. If you think my work is well done, please submit for integration this issue. Ciao and thanks again.
          Hide
          Mary Evans added a comment -

          Looks OK to me Daniele so passing this and submitting for IR

          Show
          Mary Evans added a comment - Looks OK to me Daniele so passing this and submitting for IR
          Hide
          Daniele Cordella added a comment -

          Thanks Mary!

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

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Damyon Wiese added a comment -

          Thanks Daniele, Integrated to master only.

          But - I do think it looks a little odd having the text in forms appear larger than other parts of the page.

          Show
          Damyon Wiese added a comment - Thanks Daniele, Integrated to master only. But - I do think it looks a little odd having the text in forms appear larger than other parts of the page.
          Hide
          Daniele Cordella added a comment -

          Thanks Damyon
          for you suggestion.
          I am currently focused on survey2 and I would not change my target but when I find a regression on mform I am quite forced to think to it.
          As you see I discussed a bit the solution with the excellent Mary but I really would not spend more time on it.
          If you have (or someone else has) a better solution (and I agree a larger font in mform is not a good idea, of course) please propose it. It is really welcome, here.
          Thanks in advance.

          Show
          Daniele Cordella added a comment - Thanks Damyon for you suggestion. I am currently focused on survey2 and I would not change my target but when I find a regression on mform I am quite forced to think to it. As you see I discussed a bit the solution with the excellent Mary but I really would not spend more time on it. If you have (or someone else has) a better solution (and I agree a larger font in mform is not a good idea, of course) please propose it. It is really welcome, here. Thanks in advance.
          Hide
          Mary Evans added a comment -

          The font size in Formal white is variable anyway so why worry about forms the text is a nice size and since YUI CSS sets the HTML font size (or should) then 108% is size 14pt which is good for reading forms as you do not want tiny font.

          Show
          Mary Evans added a comment - The font size in Formal white is variable anyway so why worry about forms the text is a nice size and since YUI CSS sets the HTML font size (or should) then 108% is size 14pt which is good for reading forms as you do not want tiny font.
          Hide
          Daniele Cordella added a comment -

          You are right Mary,
          but it seems that in FW the font size in mform is bigger than the same size wherever else.
          Am I wrong?

          Show
          Daniele Cordella added a comment - You are right Mary, but it seems that in FW the font size in mform is bigger than the same size wherever else. Am I wrong?
          Hide
          Mark Nelson added a comment -

          Hey guys, works as expected.

          I checked stable to compare with integration and noticed the difference. I also compared 2.4 and 2.5 to see if this needed backporting, but it does not.

          However, I did notice different spacing between each new line for the text in the question between 2.4/2.5 and master. As well as more spacing between the radio button and the radio button text in 2.5/master compared to 2.4. I assume this is expected?

          Thanks Daniele and Mary.

          Show
          Mark Nelson added a comment - Hey guys, works as expected. I checked stable to compare with integration and noticed the difference. I also compared 2.4 and 2.5 to see if this needed backporting, but it does not. However, I did notice different spacing between each new line for the text in the question between 2.4/2.5 and master. As well as more spacing between the radio button and the radio button text in 2.5/master compared to 2.4. I assume this is expected? Thanks Daniele and Mary.
          Hide
          Dan Poltawski added a comment -

          Hurrah! Thanks for your contribution - this fix is part of Moodle.

          Show
          Dan Poltawski added a comment - Hurrah! Thanks for your contribution - this fix is part of Moodle.

            People

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

              Dates

              • Created:
                Updated:
                Resolved: