Search: Use search block with icon button #155
Conversation
There was a problem hiding this comment.
👍 Kudos, this looks like a good, seamless solution. One non-blocking issue I noticed is that because the Search dropdown is treated like a modal, in that you can't scroll down the page while it's open, the scroll bar goes away, which causes a layout jump:
Screen.Capture.on.2022-02-11.at.16-39-09.mp4
|
hm, the layout jump isn't great, but it's intentional that you can't scroll, since you can't actually interact with the rest of the page until the modal is closed (because it's functionally a dialog). Is that new to this branch though? the dialog behavior is the same on trunk. |
Hmm, not new, I guess. I'm seeing it on the production test site as well. |
| The search block is inside a navigation submenu, because that provides the exact functionality the design | ||
| calls for. It also provides a consistent experience with the primary navigation menu, with respect to | ||
| keyboard navigation, ARIA states, etc. It also saves having to write custom code for all the interactions. | ||
| --> |
There was a problem hiding this comment.
Was removing the above comment intentional? It's not related to how the search block was forked, and still seems like helpful documentation to me.
There was a problem hiding this comment.
I thought it was outdated since the submenu wrapper was removed in #119, but I guess it could be referring to the nav wrapper itself? Feel free to put it back, if so.
There was a problem hiding this comment.
Yeah, I think it's there to address the confusion over why a search box would be inside a menu.
Fixes #148, fixes #99, fixes #13. This takes some of the code from #113 to use the search block itself, rather than hard-coding it with the custom HTML. Now the search form is generated by the search block, with the "use button with icon" and "button inside" settings.
The button uses the core icon, which looks the same to me, and fixes the CORS issue. Using the "button inside" format of the search lets us remove some of the button positioning code so that the button is visible again, and can get a proper focus style.
Note that the icon button doesn't currently have an accessible label - there's a PR in progress for that here: WordPress/gutenberg#38136
The spacing difference on the small make screenshot is because of a gutenberg version difference.