New search field worse on mobile, uses second row

Proposed resolution

I created a patch to add search icon into toolbar tray.

The behaviour is similar to the previous mobule versions.
The input search appears by selecting the search icon.

The icon is available for all devices: desktop, tablet and mobile.

The given patch is NOT working. While applying the patch showing error messages. Needs work.
Screenshot: apply-patch.PNG

The patch worked beautifully for me. I did not like the field in the toolbar because it was taking up too much space and causing a nice row of icons to wrap. I was about to uninstall, but your patch fixed the issue. Thank you.

I have updated the patch and able to apply, kindly check.

I worked on the failed test case of #9. The issue trigger first on this line of code. I assume that the search field initially not visible because It is only visible when we click on search filed, that's why this issue occur, So I just try solve this by adding this line of code. $page->clickLink('class-name-of-search-option');This line of code will be placed just before thisline of code.

Now the same issue occurred on this line number. So it indicates that the same function is to trigger the error after the first call of this function.

The approach I tried to overcome this issue is:-

1. With the help of wait() I tried to fill the input search filed.
2. With the help of Assert Session.
3. Clicking on serach button & fill the input filed.
4. Tried to hide and show the input field and the fill with the help of inline jQuery.
5. Tried to implement nightwatch js for the dynamic behaviour but this will be create the extra dependency on this.

Any suggestions will be appreciated.

I agree that the current solution looks quite broken on mobile. I tried to improve it in the following patch by adding the mobile search icon and toggle it on mobile, add active class, focus the search input and populate the search results.

And here is a slightly improved patch, where I moved the focus handler into a separate function for better code visibility. I just wasn't sure if I should rename the $self variable. I feel this is not so descriptive and would need a better name IMHO. Any suggestions?

Here is a new patch, changes:

- Refactored and cleaned CSS, but no major changes there, just some small improvements like stoping autocomplete popup to overflow viewport on mobile, etc,
- In JS I think we should use `once` call before anything else. Yes, we do have that `if (context != document)` but some code could still run behaviours with document context later and we would get this code executed twice,
- In JS added some helper variables, refactored and simplified code that is calling link population so there is really no need for method call there. Also moved new code for mobile search button back from the method - we are calling it only once, it is a small amount of code and IMHO I don't see a reason putting this into a method,
- In JS added `position: { collision : 'fit' }` option to autocomplete so cases where search input is close to viewport border autocomplete popup will not go outside of viewport,
- In JS some minor refactorings like " -> ' change, etc,
- In PHP just changing " to '

> I just wasn't sure if I should rename the $self variable. I feel this is not so descriptive and would need a better name IMHO.

Yeah, that is a weird name for a variable. On top of that, you don't really need that variable because you can just use `Drupal.behaviors.adminToolbarSearch` which is the same thing. I would remove that variable and just use global Drupal.behaviors.adminToolbarSearch variable for that, but this change should go into a separate issue if we want to do it.

Maybe we should add some simple test that is testing visibility and clicking of the search button on mobile?

Forgot to add screenshots of a new look.

+1 for this. Patch #14 works fine as shown in the screenshots in #15. It seems the textfield is better centered now. We should add tests for this as the behavior is changing a bit in mobile.

Needs a reroll.

I tried to rebase against 3.x but doesn't seem to fully work anymore, mobile icon doesn't show up.

Here is a brand new patch rebuild against latest 3.x.

Additionally some CSS code changes that were added are dropped now because they are not needed any more. New CSS rules are separated from the rest of the code with `#admin-toolbar-mobile-search-tab +` prefix selector so they will not affect new 'Display the search input as a menu item' option.

The new 'Display the search input as a menu item' is interesting because it is doing something similar like this patch but with using of a tray control. I am not sure should this patch improve some layout rules for this option or this should be done in some follow-up issue. Follow up issue makes more sense, maybe?

And here is the same patch against 3.0.2 security release.

Reroll again.

Patch looks good!
Moving it to RTBC.

Thanks for this.