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

      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

        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: