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

Width thresholds for form labels in the Clean Theme need to be higher.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5.1, FRONTEND
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide

      Using the Clean theme, View forms, in particular, the edit forum settings form. Inspect the width of the labels and ensure it is 180px on screen/browser that is 768px wide.

      Show
      Using the Clean theme, View forms, in particular, the edit forum settings form. Inspect the width of the labels and ensure it is 180px on screen/browser that is 768px wide.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-39608-master
    • Sprint:
      FRONTEND Sprint 1
    • Story Points (Obsolete):
      3
    • Sprint:
      FRONTEND Sprint 1

      Description

      The changes in MDL-39158 push the file picker out of place

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            phalacee Jason Fowler added a comment -

            I know on the original issue David mentioned that the @horizontalComponentOffset var wasn't used, but it is indeed used in other places else where, which is why I have left it as it is currently, while adding @horizontalComponentOffset1200 to handle the bigger size screens.

            Requesting peer review now.

            Show
            phalacee Jason Fowler added a comment - I know on the original issue David mentioned that the @horizontalComponentOffset var wasn't used, but it is indeed used in other places else where, which is why I have left it as it is currently, while adding @horizontalComponentOffset1200 to handle the bigger size screens. Requesting peer review now.
            Hide
            bawjaws David Scotson added a comment -

            I've not tested it yet but from looking at the diff the current 768 level doesn't seem to do anything and all the related code can be deleted without affecting anything as the labels are already set to that width by default.

            Show
            bawjaws David Scotson added a comment - I've not tested it yet but from looking at the diff the current 768 level doesn't seem to do anything and all the related code can be deleted without affecting anything as the labels are already set to that width by default.
            Hide
            phalacee Jason Fowler added a comment -

            Can some one please review this? I've adjusted it in response to David's comment above.

            Show
            phalacee Jason Fowler added a comment - Can some one please review this? I've adjusted it in response to David's comment above.
            Hide
            lazydaisy Mary Evans added a comment -

            @Jason, there is a problem in MDL-39820 where the margins are way too wide.

            If you look at base theme the label left margins are set to 15% which in for 960px through to 2000px is from between 144px to 300px. Compare this with Bootstrapbase where @horizontalComponantOffset768: and @horizontalComponantOffset980: sets label margins at anywhere between 220px and 265px for screen 768px to 980px respectively. Which is OTT in my estimation.

            We might just do a quick fix with MDL-39820 and wait till you have this fixed.

            Show
            lazydaisy Mary Evans added a comment - @Jason, there is a problem in MDL-39820 where the margins are way too wide. If you look at base theme the label left margins are set to 15% which in for 960px through to 2000px is from between 144px to 300px. Compare this with Bootstrapbase where @horizontalComponantOffset768: and @horizontalComponantOffset980: sets label margins at anywhere between 220px and 265px for screen 768px to 980px respectively. Which is OTT in my estimation. We might just do a quick fix with MDL-39820 and wait till you have this fixed.
            Hide
            lazydaisy Mary Evans added a comment -

            By the way what you have here looks better. I think is good to go.

            Show
            lazydaisy Mary Evans added a comment - By the way what you have here looks better. I think is good to go.
            Hide
            phalacee Jason Fowler added a comment -

            Thanks Mary, pushing for integration now then.

            Show
            phalacee Jason Fowler added a comment - Thanks Mary, pushing for integration now then.
            Hide
            gb2048 Gareth J Barnard added a comment -

            Dear Jason Fowler,

            As this is a bug will there be a MOODLE_25_STABLE branch too for the fix?

            Cheers,

            Gareth

            Show
            gb2048 Gareth J Barnard added a comment - Dear Jason Fowler , As this is a bug will there be a MOODLE_25_STABLE branch too for the fix? Cheers, Gareth
            Hide
            lazydaisy Mary Evans added a comment -

            @Gareth, they will most probably cherry-pick to M25 during integration, since it is only affecting LESS files. Guessing so anyway!

            Show
            lazydaisy Mary Evans added a comment - @Gareth, they will most probably cherry-pick to M25 during integration, since it is only affecting LESS files. Guessing so anyway!
            Hide
            gb2048 Gareth J Barnard added a comment -

            I hope so

            Show
            gb2048 Gareth J Barnard added a comment - I hope so
            Hide
            phalacee Jason Fowler added a comment -

            Gareth: yes, while 2.5 and master are sync'd, any issues for master are always backported off the master branch. They will be sync'd until 2.5.1 is released.

            Show
            phalacee Jason Fowler added a comment - Gareth: yes, while 2.5 and master are sync'd, any issues for master are always backported off the master branch. They will be sync'd until 2.5.1 is released.
            Hide
            gb2048 Gareth J Barnard added a comment -

            Dear Jason Fowler,

            Thanks - did not know that

            Cheers,

            Gareth

            Show
            gb2048 Gareth J Barnard added a comment - Dear Jason Fowler , Thanks - did not know that Cheers, Gareth
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks Jason, this has been integrated now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks Jason, this has been integrated now.
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Tested during integration review and passed.

            Show
            samhemelryk Sam Hemelryk added a comment - Tested during integration review and passed.
            Hide
            phalacee Jason Fowler added a comment -

            Thanks Sam

            Show
            phalacee Jason Fowler added a comment - Thanks Sam
            Hide
            bawjaws David Scotson added a comment -

            FYI, in Bootstrap v3 they achieve this natively, by using the standard grid for laying out forms, so you get e.g. 4/12 for the labels and 8/12 for the control (where you can choose your own values for 4&8) which expand with the container and it collapses to stacked forms at the standard grid breakpoint.

            Show
            bawjaws David Scotson added a comment - FYI, in Bootstrap v3 they achieve this natively, by using the standard grid for laying out forms, so you get e.g. 4/12 for the labels and 8/12 for the control (where you can choose your own values for 4&8) which expand with the container and it collapses to stacked forms at the standard grid breakpoint.
            Hide
            marina Marina Glancy added a comment -

            Thanks for your awesome work! This has now become a part of Moodle.

            Closing as fixed!

            Show
            marina Marina Glancy added a comment - Thanks for your awesome work! This has now become a part of Moodle. Closing as fixed!

              People

              • Votes:
                1 Vote for this issue
                Watchers:
                7 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  8/Jul/13

                  Agile