Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.5
    • Fix Version/s: 2.5
    • Component/s: Themes
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in as an Admin
      2. Select Bootstrap theme via Theme selector
      3. Float browser screen (Restore Down) and drag right side of window to left thus reducing the browser window
      4. Test that the Admin search input stays within the side block and does not overhang into gutter.
      5. Add Block 'Blog menu' and Test 'Blog search input' as above.
      6. Create a forum or move to a forum and Test 'Forum search input' as above.
      Show
      Log in as an Admin Select Bootstrap theme via Theme selector Float browser screen (Restore Down) and drag right side of window to left thus reducing the browser window Test that the Admin search input stays within the side block and does not overhang into gutter. Add Block 'Blog menu' and Test 'Blog search input' as above. Create a forum or move to a forum and Test 'Forum search input' as above.
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-38854_master

      Description

      E.g. Search forums block.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            David Scotson added a comment -

            I'm guessing this only happens in some rather unusual circumstances. The screenshot is 950x950-ish, which is not a standard resolution.

            That said, the problem can probably be fixed in the short-term by setting max-width: 100% on #adminsearchquery, #blogsearchquery, #searchform_search and any other similar search items (probably adding to where block_adminblock select is already set to 100%). Note it looks slightly worse for the #adminsearchquery as it's inside a list which nudges it over to the right.

            Actually, just trying it and it doesn't work for #searchform_search for no obvious reason. One of its containers is doing something weird so it's picking up a specific pixel width instead. Setting a width: auto on #searchform_search either fixes it or masks the problem, I'm really not sure which, the surrounding HTML is a bit obscure.

            (Random FYI: It's worth noting that in Bootstrap 3 all such form items are 100% width by default.)

            But I think there might be better options for laying these out in future if we have the opportunity to change the underlying HTML.

            Show
            David Scotson added a comment - I'm guessing this only happens in some rather unusual circumstances. The screenshot is 950x950-ish, which is not a standard resolution. That said, the problem can probably be fixed in the short-term by setting max-width: 100% on #adminsearchquery, #blogsearchquery, #searchform_search and any other similar search items (probably adding to where block_adminblock select is already set to 100%). Note it looks slightly worse for the #adminsearchquery as it's inside a list which nudges it over to the right. Actually, just trying it and it doesn't work for #searchform_search for no obvious reason. One of its containers is doing something weird so it's picking up a specific pixel width instead. Setting a width: auto on #searchform_search either fixes it or masks the problem, I'm really not sure which, the surrounding HTML is a bit obscure. (Random FYI: It's worth noting that in Bootstrap 3 all such form items are 100% width by default.) But I think there might be better options for laying these out in future if we have the opportunity to change the underlying HTML.
            Show
            David Scotson added a comment - Fix committed here: https://github.com/bmbrands/theme_bootstrap/commit/e75ba264a684c0237bff3afa2e21e7b1ea928815
            Hide
            Mary Evans added a comment -

            @David:

            I think this is a Browser thing as it also happens in Moodle too. The easy way to fix this is Zoom Out using Ctrl + Minus key (once)

            Show
            Mary Evans added a comment - @David: I think this is a Browser thing as it also happens in Moodle too. The easy way to fix this is Zoom Out using Ctrl + Minus key (once)
            Hide
            Damyon Wiese added a comment -

            Re: the resolution of the screenshot. It is just from a browser window that is not fullscreen (this is not unusual). The zoom is set to default.

            Show
            Damyon Wiese added a comment - Re: the resolution of the screenshot. It is just from a browser window that is not fullscreen (this is not unusual). The zoom is set to default.
            Hide
            David Scotson added a comment -

            It was mostly just a comment about prioritisation. Something that happens at 1024px is more important than something that happens at 1023px, even though there's only one pixel difference because it'll be seen by more people, and something that happens when your screen is less than 950 but more than 768 is less important than a bug that's always visible.

            Show
            David Scotson added a comment - It was mostly just a comment about prioritisation. Something that happens at 1024px is more important than something that happens at 1023px, even though there's only one pixel difference because it'll be seen by more people, and something that happens when your screen is less than 950 but more than 768 is less important than a bug that's always visible.
            Hide
            Michael de Raadt added a comment -

            A solution to a similar problem was provided in MDL-39201. It might be worth looking at.

            Show
            Michael de Raadt added a comment - A solution to a similar problem was provided in MDL-39201 . It might be worth looking at.
            Hide
            Bas Brands added a comment -

            I have added this to blocks.less:

            #adminsearchquery,
            #blogsearchquery,
            #searchform_search,
            .block_adminblock select

            { max-width: 100%; }

            .block_adminblock .singleselect

            { display: block; }

            #searchform_search

            { width: auto; }

            This is the first time I create a fix for something in core so I hope I'm doing it the right way

            Show
            Bas Brands added a comment - I have added this to blocks.less: #adminsearchquery, #blogsearchquery, #searchform_search, .block_adminblock select { max-width: 100%; } .block_adminblock .singleselect { display: block; } #searchform_search { width: auto; } This is the first time I create a fix for something in core so I hope I'm doing it the right way
            Hide
            Mary Evans added a comment - - edited

            Hi Bas,

            Thanks for fixing this.

            You need to add the Pull for Repository and Pull Master Branch. Also the way the compare works you need to add the following just after compare/ and before the branch name:

            master...

            See my changes above.

            Show
            Mary Evans added a comment - - edited Hi Bas, Thanks for fixing this. You need to add the Pull for Repository and Pull Master Branch. Also the way the compare works you need to add the following just after compare/ and before the branch name: master... See my changes above.
            Hide
            Mary Evans added a comment -

            Can you ADD some test instructions too?

            Thanks

            Show
            Mary Evans added a comment - Can you ADD some test instructions too? Thanks
            Hide
            Bas Brands added a comment -

            Thanks Mary!

            Okay just changed the pull from repository. I use a fork of Moodle on github to submit my fixes. My github.com/bmbrands/theme_bootstrap.git repository is not in sync with core yet.

            There are a number of small fixes that are in my repository now that could be used in core. David and I will create issues if needed or add our fixes to existing tracker issues.

            I am slowly getting used to submitting fixes, this is pretty cool!

            Show
            Bas Brands added a comment - Thanks Mary! Okay just changed the pull from repository. I use a fork of Moodle on github to submit my fixes. My github.com/bmbrands/theme_bootstrap.git repository is not in sync with core yet. There are a number of small fixes that are in my repository now that could be used in core. David and I will create issues if needed or add our fixes to existing tracker issues. I am slowly getting used to submitting fixes, this is pretty cool!
            Hide
            Mary Evans added a comment - - edited

            That's great thanks.

            As you will see I have just changed the instructions slightly. I know this may sound daft, but you have to write is as though the person testing is a complete dummy!

            Just making sure so that I have this correct...in the instructions you wrote the test included going into a Forum and testing that the Blog search displays ok? Or did you mean to write Forum search? Just wondering?

            Show
            Mary Evans added a comment - - edited That's great thanks. As you will see I have just changed the instructions slightly. I know this may sound daft, but you have to write is as though the person testing is a complete dummy! Just making sure so that I have this correct...in the instructions you wrote the test included going into a Forum and testing that the Blog search displays ok? Or did you mean to write Forum search? Just wondering?
            Hide
            Mary Evans added a comment -

            With regards to your bootstrap repo in GitHub, this in no longer used and should NOT be used, becasue Bootstrap is part of Moodle core.

            To make changes you must ALWAYS use git://github/bmbrands/moodle.git from now on. If you get stuck just ask!

            Show
            Mary Evans added a comment - With regards to your bootstrap repo in GitHub, this in no longer used and should NOT be used, becasue Bootstrap is part of Moodle core. To make changes you must ALWAYS use git://github/bmbrands/moodle.git from now on. If you get stuck just ask!
            Hide
            Mary Evans added a comment -

            OK...just looking at the patch you submitted.
            Would it not be better if...

            #searchform_search {
              width: auto;
            }
            

            ...came before this rule?

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

            So that the cascade would start at width: auto and stop at 92% otherwise Auto may take it beyond?

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

            Show
            Mary Evans added a comment - OK...just looking at the patch you submitted. Would it not be better if... #searchform_search { width: auto; } ...came before this rule? #adminsearchquery, #blogsearchquery, #searchform_search, .block_adminblock select { width: 100%; max-width: 92%; } So that the cascade would start at width: auto and stop at 92% otherwise Auto may take it beyond? #searchform_search { width: auto; }   #adminsearchquery, #blogsearchquery, #searchform_search, .block_adminblock select { width: 100%; max-width: 92%; }
            Hide
            Bas Brands added a comment -

            Cool,

            Okay, thanks for pointing that out (about the instructions). The fix is for admin, forum and blog search. So each of these need to be tested.

            Good to know I'm doing the changes the right way using a forked moodle repository

            Show
            Bas Brands added a comment - Cool, Okay, thanks for pointing that out (about the instructions). The fix is for admin, forum and blog search. So each of these need to be tested. Good to know I'm doing the changes the right way using a forked moodle repository
            Hide
            Mary Evans added a comment -

            Which is how I explained originally when adding the Bootstrap theme, but was told differently!

            Goes to show you are never too old to learn.

            Just be sure to keep this forked branch up-to-date each week or each time notifications come through to say that Moodle was updated.

            Show
            Mary Evans added a comment - Which is how I explained originally when adding the Bootstrap theme, but was told differently! Goes to show you are never too old to learn. Just be sure to keep this forked branch up-to-date each week or each time notifications come through to say that Moodle was updated.
            Hide
            Mary Evans added a comment -

            Bas, I have just set this for Integration. I think you must have forgotten to do this!

            Show
            Mary Evans added a comment - Bas, I have just set this for Integration. I think you must have forgotten to do this!
            Hide
            Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now

            Show
            Sam Hemelryk added a comment - Thanks guys, this has been integrated now
            Hide
            David Monllaó added a comment -

            It passes, all working as expected

            Show
            David Monllaó added a comment - It passes, all working as expected
            Hide
            Eloy Lafuente (stronk7) added a comment -

            I feel myself really alone tonight! So was time to push your fixes upstream!

            "Lest we forget. We will remember them."

            Thanks and ciao!

            Show
            Eloy Lafuente (stronk7) added a comment - I feel myself really alone tonight! So was time to push your fixes upstream! "Lest we forget. We will remember them." Thanks and ciao!

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: