Details

    • Testing Instructions:
      Hide
      1. Select Clean theme.
      2. With editing on in Home page ADD a login block.
      3. Turn editing off and log out.
      4. With the front page still open, TEST that the Login block, 'username' and 'password', input boxes do not overhang into the gutter between side blocks and main content (side-pre), or side blocks and page edge (side-post).
      5. Continue testing when resizing screen on desktop, also test on mobile devices, tablet, laptop etc...if possible.
      6. TEST for regressions (there should be none) using an RTL language, like Hebrew (he) for example.
      Show
      Select Clean theme. With editing on in Home page ADD a login block. Turn editing off and log out. With the front page still open, TEST that the Login block, 'username' and 'password', input boxes do not overhang into the gutter between side blocks and main content (side-pre), or side blocks and page edge (side-post). Continue testing when resizing screen on desktop, also test on mobile devices, tablet, laptop etc...if possible. TEST for regressions (there should be none) using an RTL language, like Hebrew (he) for example.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE, MOODLE_26_STABLE
    • Pull Master Branch:

      Description

      Hi,
      the text fields are too wide when the side gets smaller. The width is larger than the width of the block width. The width of the text fields should be 80 or 90 percent of the block width.
      Best regards, Ralf

        Gliffy Diagrams

        1. 2014_02_07_22_13_29_Moodle_GJB.png
          13 kB
        2. MDL-40938.png
          27 kB
        3. MDL-40938-login.jpg
          9 kB
        4. MDL-40938-search.jpg
          7 kB

          Activity

          Hide
          lazydaisy Mary Evans added a comment -

          Thank you for reporting this bug.
          Let's see if we can improve this.

          Show
          lazydaisy Mary Evans added a comment - Thank you for reporting this bug. Let's see if we can improve this.
          Hide
          lazydaisy Mary Evans added a comment -

          Looks like I missed this one. Will fix it this weekend.

          Show
          lazydaisy Mary Evans added a comment - Looks like I missed this one. Will fix it this weekend.
          Hide
          lazydaisy Mary Evans added a comment -

          Hi Gareth, any chance you can peer review this for master branch only so that if it works/looks OK I can then create branches to fix 2.6 and 2.5?
          Thanks
          Mary

          Show
          lazydaisy Mary Evans added a comment - Hi Gareth, any chance you can peer review this for master branch only so that if it works/looks OK I can then create branches to fix 2.6 and 2.5? Thanks Mary
          Hide
          cibot CiBoT added a comment -

          Results for MDL-40938

          • Remote repository: git://github.com/lazydaisy/moodle.git
          Show
          cibot CiBoT added a comment - Results for MDL-40938 Remote repository: git://github.com/lazydaisy/moodle.git Remote branch MDL-40938 -25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1151 Error: Unable to fetch information from MDL-40938 -25 branch at git://github.com/lazydaisy/moodle.git. Remote branch MDL-40938 -26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1152 Error: Unable to fetch information from MDL-40938 -26 branch at git://github.com/lazydaisy/moodle.git. Remote branch MDL-40938 -27 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1153 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1153/artifact/work/smurf.html
          Hide
          gb2048 Gareth J Barnard added a comment -

          Dear Mary Evans,

          Ok, will do. Interesting area, I was having a discussion with Sam Hemelryk about the size of the search box which has the same issue but was not equally spaced on both sides. That solution was a 92% width. I cannot remember the issue though.

          Gareth

          Show
          gb2048 Gareth J Barnard added a comment - Dear Mary Evans , Ok, will do. Interesting area, I was having a discussion with Sam Hemelryk about the size of the search box which has the same issue but was not equally spaced on both sides. That solution was a 92% width. I cannot remember the issue though. Gareth
          Hide
          gb2048 Gareth J Barnard added a comment - - edited

          Hi Mary Evans,

          It does work. I was worried about the 'max-width' being a fixed pixel one, but that seems to be fine, even when zooming out on IE11. However, I consider for consistency that the with should be 92% in line with the search box, even though the two are not seen together. And for future maintenance consistency, in 'blocks.less', remove:

          .block_login input#login_username,
          .block_login input#login_password {
              width: 95%;
          }
          

          and where the search box is defined:

          #adminsearchquery,
          #blogsearchquery,
          #searchform_search,
          .block_adminblock select {
              max-width: 92%;
          }
          

          have:

          #adminsearchquery,
          #blogsearchquery,
          #searchform_search,
          .block_adminblock select,
          .block_login input#login_username,
          .block_login input#login_password {
              max-width: 92%;
          }
          

          I don't think the change to 'max-width' should make a difference here as we are still retaining the '.block_login .content' attribute definitions.

          [N] Syntax - Indentation should be four spaces - http://docs.moodle.org/dev/CSS_coding_style#Block_Style
          [Y] Whitespace
          [-] Output
          [-] Language
          [-] Databases
          [N] Testing (instructions and automated tests) - Add RTL.
          [-] Security
          [-] Documentation
          [Y] Git
          [-] Third party code
          [N] Sanity check - see comment above.

          Cheers,

          Gareth

          Show
          gb2048 Gareth J Barnard added a comment - - edited Hi Mary Evans , It does work. I was worried about the 'max-width' being a fixed pixel one, but that seems to be fine, even when zooming out on IE11. However, I consider for consistency that the with should be 92% in line with the search box, even though the two are not seen together. And for future maintenance consistency, in 'blocks.less', remove: .block_login input#login_username, .block_login input#login_password { width: 95%; } and where the search box is defined: #adminsearchquery, #blogsearchquery, #searchform_search, .block_adminblock select { max-width: 92%; } have: #adminsearchquery, #blogsearchquery, #searchform_search, .block_adminblock select, .block_login input#login_username, .block_login input#login_password { max-width: 92%; } I don't think the change to 'max-width' should make a difference here as we are still retaining the '.block_login .content' attribute definitions. [N] Syntax - Indentation should be four spaces - http://docs.moodle.org/dev/CSS_coding_style#Block_Style [Y] Whitespace [-] Output [-] Language [-] Databases [N] Testing (instructions and automated tests) - Add RTL. [-] Security [-] Documentation [Y] Git [-] Third party code [N] Sanity check - see comment above. Cheers, Gareth
          Hide
          lazydaisy Mary Evans added a comment - - edited

          Hi Gareth,
          Thanks for reviewing this.

          I got the syntax wrong! My fault as I was messing with compiled LESS css. Copy paste from moodle.css to my patch branch. Good job you spotted it.

          As for the max-width: 92% well that is not really doing anything other than restraining it in a block. However, this keeps it in check in a wider area...
           

          input, textarea, .uneditable-input {
              width: 206px;
          }
          

          My decision to use a 95% for the input boxes is this...as you can see here the login input boxes are perfectly aligned with the 'Remember me' checkbox and also sits neatly within the content padding.

          Whereas the search box sits inside the padding with a few pixels either side, but then there is nothing to disturb the tranquillity of its surroundings...it looks perfect.

          So we have two unique settings for two distinct input boxes.

          By the way, the content width is the same as the .block .minicalendar which is in 'calendar.less' whereas mine is in blocks.less, but I could easily remove it and add it to blocks.less and tag .block_login .content to it like so...
           

          .block .minicalendar,
          .block_login .content {
              max-width: 280px;
              margin-left: auto;
              margin-right: auto;
          }
          

          Which keeps things together.

          Show
          lazydaisy Mary Evans added a comment - - edited Hi Gareth, Thanks for reviewing this. I got the syntax wrong! My fault as I was messing with compiled LESS css. Copy paste from moodle.css to my patch branch. Good job you spotted it. As for the max-width: 92% well that is not really doing anything other than restraining it in a block. However, this keeps it in check in a wider area...   input, textarea, .uneditable-input { width: 206px; } My decision to use a 95% for the input boxes is this...as you can see here the login input boxes are perfectly aligned with the 'Remember me' checkbox and also sits neatly within the content padding. Whereas the search box sits inside the padding with a few pixels either side, but then there is nothing to disturb the tranquillity of its surroundings...it looks perfect. So we have two unique settings for two distinct input boxes. By the way, the content width is the same as the .block .minicalendar which is in 'calendar.less' whereas mine is in blocks.less, but I could easily remove it and add it to blocks.less and tag .block_login .content to it like so...   .block .minicalendar, .block_login .content { max-width: 280px; margin-left: auto; margin-right: auto; } Which keeps things together.
          Hide
          gb2048 Gareth J Barnard added a comment - - edited

          Hi Mary,

          For interest, see '2014_02_07_22_13_29_Moodle_GJB.png' as I'm not getting the 'Remember username' issue. Chrome 32. EDIT - When setting the max-width to 92%.

          Show
          gb2048 Gareth J Barnard added a comment - - edited Hi Mary, For interest, see '2014_02_07_22_13_29_Moodle_GJB.png' as I'm not getting the 'Remember username' issue. Chrome 32. EDIT - When setting the max-width to 92%.
          Hide
          lazydaisy Mary Evans added a comment -

          Who said the Remember username was an issue?

          Show
          lazydaisy Mary Evans added a comment - Who said the Remember username was an issue?
          Hide
          lazydaisy Mary Evans added a comment -

          What's with the RTL test instructions? There is nothing that affect RTL in this patch so no need to add any styles they already exist.

          Show
          lazydaisy Mary Evans added a comment - What's with the RTL test instructions? There is nothing that affect RTL in this patch so no need to add any styles they already exist.
          Hide
          gb2048 Gareth J Barnard added a comment -

          Hi Mary,

          RE: Remember username.

          Umm, you said in "My decision to use a 95% for the input boxes is this...as you can see here the login input boxes are perfectly aligned with the 'Remember me' checkbox" but called it 'Remember me'. It was in relation to deciding on 95% as against 92% but my screen shot '2014_02_07_22_13_29_Moodle_GJB.png' shows that this is not an issue when changing to 92% when making consistently the same as the search box.

          RE: RTL.

          The thing is even though there is no RTL in the patch, the patch affects an element of functionality that is affected by RTL. Therefore an RTL test is required to demonstrate that the change does not affect RTL and cause a regression. I have often been gob smacked in the past by totally unexpected implications of a change that affect another thing even when from a structured walk-through of the code you would not conceive it happening.

          It could have been that the fix caused the boxes to overflow when used in RTL even though it does not. But that has been demonstrated by checking with RTL.

          Cheers,

          Gareth

          Show
          gb2048 Gareth J Barnard added a comment - Hi Mary, RE: Remember username. Umm, you said in "My decision to use a 95% for the input boxes is this...as you can see here the login input boxes are perfectly aligned with the 'Remember me' checkbox" but called it 'Remember me'. It was in relation to deciding on 95% as against 92% but my screen shot '2014_02_07_22_13_29_Moodle_GJB.png' shows that this is not an issue when changing to 92% when making consistently the same as the search box. RE: RTL. The thing is even though there is no RTL in the patch, the patch affects an element of functionality that is affected by RTL. Therefore an RTL test is required to demonstrate that the change does not affect RTL and cause a regression. I have often been gob smacked in the past by totally unexpected implications of a change that affect another thing even when from a structured walk-through of the code you would not conceive it happening. It could have been that the fix caused the boxes to overflow when used in RTL even though it does not. But that has been demonstrated by checking with RTL. Cheers, Gareth
          Hide
          lazydaisy Mary Evans added a comment -

          Lets talk about aesthetics. If I use 92% the login inputs look lopsided, and so to compensate I would end up having to add a fixed margin of say 2px or 3px to the left to straighten it up so that it looks neat. This then sends me into RTL land as margin-left would need to be margin-right, and so on, it all gets messy...for what? This lopsidedness is not evident on the search input as that sits on its own. So my real point in all this is if anything we should consider changing the search input to 95%.

          Show
          lazydaisy Mary Evans added a comment - Lets talk about aesthetics. If I use 92% the login inputs look lopsided, and so to compensate I would end up having to add a fixed margin of say 2px or 3px to the left to straighten it up so that it looks neat. This then sends me into RTL land as margin-left would need to be margin-right, and so on, it all gets messy...for what? This lopsidedness is not evident on the search input as that sits on its own. So my real point in all this is if anything we should consider changing the search input to 95%.
          Hide
          lazydaisy Mary Evans added a comment -

          I have added the Test instruction for RTL and fixed the syntax of the original CSS. However, when LESS files are compiled without compressing, moodle.css makes a mockery of Moodle Docs CSS guidelines!

          Show
          lazydaisy Mary Evans added a comment - I have added the Test instruction for RTL and fixed the syntax of the original CSS. However, when LESS files are compiled without compressing, moodle.css makes a mockery of Moodle Docs CSS guidelines!
          Hide
          lazydaisy Mary Evans added a comment -

          Hi Gareth, can you check Peer Review this again or pass it on if you are hard pressed for time to do this today.

          Show
          lazydaisy Mary Evans added a comment - Hi Gareth, can you check Peer Review this again or pass it on if you are hard pressed for time to do this today.
          Hide
          cibot CiBoT added a comment -
          Show
          cibot CiBoT added a comment - Results for MDL-40938 Remote repository: git://github.com/lazydaisy/moodle.git Remote branch MDL-40938 -25 to be integrated into upstream MOODLE_25_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1157 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1157/artifact/work/smurf.html Remote branch MDL-40938 -26 to be integrated into upstream MOODLE_26_STABLE Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1158 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1158/artifact/work/smurf.html Remote branch MDL-40938 -27 to be integrated into upstream master Executed job http://integration.moodle.org/job/Precheck%20remote%20branch/1159 Details: http://integration.moodle.org/job/Precheck%20remote%20branch/1159/artifact/work/smurf.html
          Hide
          gb2048 Gareth J Barnard added a comment -

          Hi Mary,

          With the whole 92 / 95% thing I think that 95% looks lop sided! Anyway, I do not wish to nit pick over 3% unless its the interest rate on savings.

          In the long term for all it would be better if the widths were 100% with a fixed small margin on either side. Then we would not have disparity across browsers floating point calculations.

          There is nothing we can do about LESS -> CSS and in fact 'recess' is stuck on LESS 1.3, whilst 'lessc' has been updated to LESS 1.6. Ho hum, will see how things go.

          Give me a mo to peer review, internet has being playing up like a child with a tantrum.

          Cheers,

          Gareth

          Show
          gb2048 Gareth J Barnard added a comment - Hi Mary, With the whole 92 / 95% thing I think that 95% looks lop sided! Anyway, I do not wish to nit pick over 3% unless its the interest rate on savings. In the long term for all it would be better if the widths were 100% with a fixed small margin on either side. Then we would not have disparity across browsers floating point calculations. There is nothing we can do about LESS -> CSS and in fact 'recess' is stuck on LESS 1.3, whilst 'lessc' has been updated to LESS 1.6. Ho hum, will see how things go. Give me a mo to peer review, internet has being playing up like a child with a tantrum. Cheers, Gareth
          Hide
          gb2048 Gareth J Barnard added a comment -

          Noticed that 27 branch has two commits:

          lazydaisy	MDL-40938 theme_bootstrap: Fixed text fields in login block.	154d900
          Mary Evans
          lazydaisy	MDL-40938 theme_bootstrapbase: Fix for text fields in login block.	efdfba2

          And the second is wrong with 'theme_bootstrap'.

          Show
          gb2048 Gareth J Barnard added a comment - Noticed that 27 branch has two commits: lazydaisy MDL-40938 theme_bootstrap: Fixed text fields in login block. 154d900 Mary Evans lazydaisy MDL-40938 theme_bootstrapbase: Fix for text fields in login block. efdfba2 And the second is wrong with 'theme_bootstrap'.
          Hide
          gb2048 Gareth J Barnard added a comment -

          [Y] Syntax Observation note: "// Overide for login block" has no full stop at end so Code Checker would complain.
          [Y] Whitespace
          [-] Output
          [-] Language
          [-] Databases
          [Y] Testing (instructions and automated tests).
          [-] Security
          [-] Documentation
          [N] Git: Fix two commits in MDL-40938-27 branch.
          [-] Third party code
          [Y] Sanity check. NOTE: Need second opinion on 95% thing at integration review.

          If you get a chance to fix the full stop in the comment, do so. I don't see it as important as '//' comments in LESS do not make it into the CSS. All depends on the integrator to make the call from a HQ point of view of implementing standards.

          Show
          gb2048 Gareth J Barnard added a comment - [Y] Syntax Observation note: "// Overide for login block" has no full stop at end so Code Checker would complain. [Y] Whitespace [-] Output [-] Language [-] Databases [Y] Testing (instructions and automated tests). [-] Security [-] Documentation [N] Git: Fix two commits in MDL-40938 -27 branch. [-] Third party code [Y] Sanity check. NOTE: Need second opinion on 95% thing at integration review. If you get a chance to fix the full stop in the comment, do so. I don't see it as important as '//' comments in LESS do not make it into the CSS. All depends on the integrator to make the call from a HQ point of view of implementing standards.
          Hide
          lazydaisy Mary Evans added a comment -

          Thanks...

          Show
          lazydaisy Mary Evans added a comment - Thanks...
          Hide
          cibot CiBoT added a comment -

          Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!

          Show
          cibot CiBoT added a comment - Moving this issue to current integration cycle, will be reviewed soon. Thanks for the hard work!
          Hide
          samhemelryk Sam Hemelryk added a comment -

          Thanks Mary - this has been integrated now.

          As a note I'm not really 100% sold on using .content here, it works perfectly for the login block, but for most blocks you'd have to find a more specific container relating to just the elements.
          But then this is a clear improvement and is perfect for the login block so +1 it goes in
          Gareth mentioned that we hit a similar issue when cleaning up block code, this sort of solution I think may be exactly what we are looking for. We should come up with a specific container class we can apply to an element in a block that limits like this, and then wrap inputs in blocks in this container where required.
          From memory that includes the admin block search, the forum block search and a couple of other blocks.
          92% that we are using at the moment really isn't ideal and it'd be great to have a consistent approach taken.

          Cheers
          Sam

          Show
          samhemelryk Sam Hemelryk added a comment - Thanks Mary - this has been integrated now. As a note I'm not really 100% sold on using .content here, it works perfectly for the login block, but for most blocks you'd have to find a more specific container relating to just the elements. But then this is a clear improvement and is perfect for the login block so +1 it goes in Gareth mentioned that we hit a similar issue when cleaning up block code, this sort of solution I think may be exactly what we are looking for. We should come up with a specific container class we can apply to an element in a block that limits like this, and then wrap inputs in blocks in this container where required. From memory that includes the admin block search, the forum block search and a couple of other blocks. 92% that we are using at the moment really isn't ideal and it'd be great to have a consistent approach taken. Cheers Sam
          Hide
          lazydaisy Mary Evans added a comment -

          Thanks Sam

          Show
          lazydaisy Mary Evans added a comment - Thanks Sam
          Hide
          rwijaya Rossiani Wijaya added a comment -

          This is working as expected.

          Tested for 2.5, 2.6 and master.

          Test passed.

          Show
          rwijaya Rossiani Wijaya added a comment - This is working as expected. Tested for 2.5, 2.6 and master. Test passed.
          Hide
          lazydaisy Mary Evans added a comment -

          Thanks Rossi

          Show
          lazydaisy Mary Evans added a comment - Thanks Rossi
          Hide
          stronk7 Eloy Lafuente (stronk7) added a comment -

          Fetch your remotes, prune them,
          clean your integrated branches and say "Ahem".

          Rebase your ongoing stuff, keep conflicts away
          don't forget to test the code and we'll love you again.

          Thanks, closing!

          Show
          stronk7 Eloy Lafuente (stronk7) added a comment - Fetch your remotes, prune them, clean your integrated branches and say "Ahem". Rebase your ongoing stuff, keep conflicts away don't forget to test the code and we'll love you again. Thanks, closing!

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:
                Fix Release Date:
                14/May/13