feat(mc): Implement search suggestions by reusing contentSearchUI #2611
Conversation
| // Trigger initialization of ContentSearch in case it hasn't happened yet | ||
| dispatchEvent(new CustomEvent("ContentSearchClient", {detail: {}})); | ||
| componentDidMount() { | ||
| let searchText = document.getElementById("search-input"); |
k88hudson
May 18, 2017
Member
I would use a ref for this instead of an id
I would use a ref for this instead of an id
|
This is great! I am all for avoiding duplication of code I think the only thing needed here is a bit of polish on the edge where the suggestions touch the bottom of the input (it looks a little rough at the moment) |
Mardak
changed the title
|
Looks good to me – the only change is that the square corner should only be applied when |
| @@ -184,7 +184,6 @@ | |||
| input { | |||
| @include search-input; | |||
| border-top-left-radius: $search-border-radius; | |||
| border-bottom-left-radius: $search-border-radius; | |||
k88hudson
May 24, 2017
Member
This should be rounded, and only 0 when suggestions are visible
This should be rounded, and only 0 when suggestions are visible
Mardak
May 24, 2017
Author
Member
Oh indeed! Nice catch.
Oh indeed! Nice catch.
Mardak
May 25, 2017
Author
Member
I changed this to use the 4-in-1 border-radius to avoid issues with accidental ltr partial border styles affecting rtl partial border styles. In this case it would have been okay as the ltr "expanded" bottom-left being 0 happens to match the rtl non-"expanded" bottom-left already being 0.
I changed this to use the 4-in-1 border-radius to avoid issues with accidental ltr partial border styles affecting rtl partial border styles. In this case it would have been okay as the ltr "expanded" bottom-left being 0 happens to match the rtl non-"expanded" bottom-left already being 0.
| @@ -184,7 +184,6 @@ | |||
| input { | |||
| @include search-input; | |||
| border-top-left-radius: $search-border-radius; | |||
| border-bottom-left-radius: $search-border-radius; | |||
| padding-inline-start: 35px; | |||
|
|
|||
| &:focus + button { | |||
Mardak
May 25, 2017
Author
Member
Ah doh. I thought I didn't need to change button to .search-button here as it's effectively scoped to just the actual search button, but CSS specificity calculations with the element caused styling to get messed up relative to the lower specificity of the class selector.
Ah doh. I thought I didn't need to change button to .search-button here as it's effectively scoped to just the actual search button, but CSS specificity calculations with the element caused styling to get messed up relative to the lower specificity of the class selector.
Fix #2517. I was looking into implementing suggestions, and noticed
contentSearchUIhas all sorts of fancy/featureful/accessible/etc code. Any particular reason we shouldn't reuse all that logic that powers the existing newtab and home pages? It's not the UI as designed for activity-stream and just plain html as opposed to React. ni?@k88hudsonHere's a webm/gif video of a search in activity-stream that can be hooked up with just a few lines adding the script, css, and calling the constructor:
https://people-mozilla.org/~elee/activity-stream/activity-stream.search.webm
If you're not familiar with the keyboard behavior, you can press down to select items (form history as well as search suggestions) and also press alt-down to select a one-off search.
Updated PR with UI tweaks and tests and other cleanup:
