Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-39191

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

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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.

        Gliffy Diagrams

          Attachments

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

            Activity

            Hide
            poltawski 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
            poltawski 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
            daniss 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
            daniss 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
            daniss 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
            daniss 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
            lazydaisy 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
            lazydaisy 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
            daniss 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
            daniss 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
            lazydaisy Mary Evans added a comment -

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

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

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

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

            sorry is this master only?

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - sorry is this master only?
            Hide
            daniss 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
            daniss 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
            daniss 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
            daniss 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
            stronk7 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
            stronk7 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
            dougiamas Martin Dougiamas added a comment -

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

            Show
            dougiamas 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 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 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
            dmonllao 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
            dmonllao 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
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

            Closing as fixed, ciao

            Show
            stronk7 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:
                  Fix Release Date:
                  14/May/13