Moodle
  1. Moodle
  2. MDL-39608

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major 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
    • Story Points:
      3
    • Rank:
      50298
    • Sprint:
      FRONTEND Sprint 1

      Description

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

        Issue Links

          Activity

          Hide
          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
          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
          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
          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
          Jason Fowler added a comment -

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

          Show
          Jason Fowler added a comment - Can some one please review this? I've adjusted it in response to David's comment above.
          Hide
          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
          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
          Mary Evans added a comment -

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

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

          Thanks Mary, pushing for integration now then.

          Show
          Jason Fowler added a comment - Thanks Mary, pushing for integration now then.
          Hide
          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
          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
          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
          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
          Gareth J Barnard added a comment -

          I hope so

          Show
          Gareth J Barnard added a comment - I hope so
          Hide
          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
          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
          Gareth J Barnard added a comment -

          Dear Jason Fowler,

          Thanks - did not know that

          Cheers,

          Gareth

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

          Thanks Jason, this has been integrated now.

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

          Tested during integration review and passed.

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

          Thanks Sam

          Show
          Jason Fowler added a comment - Thanks Sam
          Hide
          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
          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 Glancy added a comment -

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

          Closing as fixed!

          Show
          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:

                Agile