Moodle

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

Details

  • Type: Bug Bug
  • Status: Closed 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
  • Pull Master Diff URL:

Description

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

Issue Links

is a regression caused by

Sub-task - Issues are sometimes broken into multiple sub-tasks. MDL-39158 Forms in bootstrap are not wide enough on big screens!

  • Minor - Minor loss of function where workaround is possible
  • Closed - The issue is considered finished, the resolution is correct. Issues which are closed should not be reopened.
will help resolve

Epic - A big user story that needs to be broken down. MDL-38856 META: Issues around Bootstrapbase and Clean theme

  • Critical - Crashes server, loss of data, severe memory leak
  • Open - The issue is open and ready for the assignee to start work on it.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
Gareth J Barnard added a comment -

I hope so

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

Tested during integration review and passed.

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

Thanks Sam

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

Vote (1)
Watch (7)

Dates

  • Created:
    Updated:
    Resolved:

Agile