Moodle
  1. Moodle
  2. MDL-39191

Groups of mform checkboxes and radio buttons are misaligned in all "base" based themes

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.3
    • Fix Version/s: 2.5
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      Copy the attached test.php file to your moodle root and visit it.
      Without and with the patch you should get the same I had in problem.png and expected.png.

      Do some exploratory testing on different pages in Moodle with forms that have radio buttons and checkboxes on the same row. Test in multiple themes. Look for issues with the vertical alignment of the labels and the checkbox/radio buttons.

      Show
      Copy the attached test.php file to your moodle root and visit it. Without and with the patch you should get the same I had in problem.png and expected.png. Do some exploratory testing on different pages in Moodle with forms that have radio buttons and checkboxes on the same row. Test in multiple themes. Look for issues with the vertical alignment of the labels and the checkbox/radio buttons.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-39191_master
    • Rank:
      49794

      Description

      It is some time I see misaligned elements in the frame of moodle forms.
      Attached are two screen shots describing the problem and the expected behaviour.

      1. test.php
        2 kB
        Daniele Cordella
      1. expected.png
        21 kB
      2. problem.png
        21 kB
      3. splash_linux.png
        14 kB
      4. splash_win7_IE9.png
        11 kB

        Activity

        Hide
        Dan Poltawski added a comment -

        Hi Danielle,

        Looking into this, it seems to be an issue which affects all the canavas based themes? Perhaps it would be better to fix it in canvas or base, rather than just in formal_white?

        Show
        Dan Poltawski added a comment - Hi Danielle, Looking into this, it seems to be an issue which affects all the canavas based themes? Perhaps it would be better to fix it in canvas or base, rather than just in formal_white?
        Hide
        Daniele Cordella added a comment -

        Yes Dan, it seems you are right.
        A general solution is always better than a local one.
        I agree with you!

        Show
        Daniele Cordella added a comment - Yes Dan, it seems you are right. A general solution is always better than a local one. I agree with you!
        Hide
        Daniele Cordella added a comment -

        I added Mary as watcher because as Dan pointed out, this seems to be a general theme issue and not a local formal_white issue. So I leave the floor to Mary.

        Show
        Daniele Cordella added a comment - I added Mary as watcher because as Dan pointed out, this seems to be a general theme issue and not a local formal_white issue. So I leave the floor to Mary.
        Hide
        Mary Evans added a comment -

        Ciao Daniele,

        You can fix this in theme/base/style/core.css for the mform labels and such. Thanks

        Mary

        Show
        Mary Evans added a comment - Ciao Daniele, You can fix this in theme/base/style/core.css for the mform labels and such. Thanks Mary
        Hide
        Daniele Cordella added a comment -

        I submit again for peer review as the change has been moved from theme/formal_white/style/core.css to theme/base/style/core.css as of the suggestion of Mary.
        I already tested it locally and all works fine.

        Show
        Daniele Cordella added a comment - I submit again for peer review as the change has been moved from theme/formal_white/style/core.css to theme/base/style/core.css as of the suggestion of Mary. I already tested it locally and all works fine.
        Hide
        Mary Evans added a comment -

        +1 from me. Looks and works OK for me too.

        Show
        Mary Evans added a comment - +1 from me. Looks and works OK for me too.
        Hide
        Mary Evans added a comment -

        It's set for Integration review now so I guess it's in the list for next PULL

        Show
        Mary Evans added a comment - It's set for Integration review now so I guess it's in the list for next PULL
        Hide
        Eloy Lafuente (stronk7) added a comment -

        sorry is this master only?

        Show
        Eloy Lafuente (stronk7) added a comment - sorry is this master only?
        Hide
        Daniele Cordella added a comment -

        You are right Eloy. I should be for m24 and 23 too.
        I'll add a branch in minutes.

        Show
        Daniele Cordella added a comment - You are right Eloy. I should be for m24 and 23 too. I'll add a branch in minutes.
        Hide
        Daniele Cordella added a comment -

        No Eloy, no need of intervention on M23 and 24. The issue is not present.
        As far as I can understand, this patch is only for master and ready for integration review.
        Ciao.

        Show
        Daniele Cordella added a comment - No Eloy, no need of intervention on M23 and 24. The issue is not present. As far as I can understand, this patch is only for master and ready for integration review. Ciao.
        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
        Martin Dougiamas added a comment -

        This looks completely non-controversial and I will +10 it for an Indiana Jones landing in 2.5!

        Show
        Martin Dougiamas added a comment - This looks completely non-controversial and I will +10 it for an Indiana Jones landing in 2.5!
        Hide
        Damyon Wiese added a comment -

        Thanks Daniele, this has been integrated to master now. I did some testing in a range of different pages in different themes to make sure this didn't make any of the other themes look worse (and it looked OK to me). I'll add a bit more to the testing instructions to get the tester to do some more of the same.

        Show
        Damyon Wiese added a comment - Thanks Daniele, this has been integrated to master now. I did some testing in a range of different pages in different themes to make sure this didn't make any of the other themes look worse (and it looked OK to me). I'll add a bit more to the testing instructions to get the tester to do some more of the same.
        Hide
        David Monllaó added a comment -

        It passes. Depending on the zoom in some themes it seems that the checkboxes are slightly above the label but changing the zoom all looks nice. I've also did a bit of exploratory test.

        Attaching two curious differences between win7-IE9 & linux-firefox.

        Show
        David Monllaó added a comment - It passes. Depending on the zoom in some themes it seems that the checkboxes are slightly above the label but changing the zoom all looks nice. I've also did a bit of exploratory test. Attaching two curious differences between win7-IE9 & linux-firefox.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        One of the very last fixes before 2.5, well done!

        Closing as fixed, ciao

        Show
        Eloy Lafuente (stronk7) added a comment - One of the very last fixes before 2.5, well done! Closing as fixed, ciao

          People

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

            Dates

            • Created:
              Updated:
              Resolved: