Template Parts: Add search to replacement modal #42459
Conversation
Mamaduka
added
[Type] Enhancement
|
@ntsekouras, @jorgefilipecosta, sorry for the pings, but I would appreciate your feedback here since you both worked on similar modals. |
| export function searchItems( items, searchValue ) { | ||
| if ( ! searchValue ) { | ||
| return items; | ||
| } | ||
|
|
||
| const normalizedSearchValue = searchValue.toLowerCase(); | ||
| return items.filter( ( item ) => { | ||
| const normalizedTitle = item.title.toLowerCase(); | ||
|
|
||
| return normalizedTitle.includes( normalizedSearchValue ); | ||
| } ); |
There was a problem hiding this comment.
This is a very basic search. I'm not sure if we need a ranked search like inserter here.
There was a problem hiding this comment.
I think it's ok to start with something simple, but also looks like it probably wouldn't be too difficult to improve it. I had a look at the code for the inserter to see how it works: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/inserter/search-items.js#L145-L161
The biggest win would probably be to still consider it a match if all of the search term words are in the title but order doesn't matter (e.g. 'large dark' and 'dark large' produce the same search results), which seems to be what this bit of code does using the words function:
gutenberg/packages/block-editor/src/components/inserter/search-items.js
Lines 144 to 162 in dfc4b8b
It'll be a bit easier for patterns since there's only one title field to check against.
edit: Also just saw that lodash words is about to be replaced in #42467.
There was a problem hiding this comment.
This is a very basic search. I'm not sure if we need a ranked search like inserter here.
We don't actually have a ranking system right now and the one we need would be different than the inserter, that uses frecency.
Having said that, I think we can start small with a couple of tweaks maybe:
- Rename the function to something more specific(ex.
searchPatterns) as is not generic as some other similar functions - Probably add simple
accents, trimhandling tonormalizedTitle.
In general I think integrating pattern explorer here would be fitting, but until we have something like that, it's okay to have at least a basic search by title. Also when we'll work on a patterns ranking system we might convert similar functions to a selector..
There was a problem hiding this comment.
We don't actually have a ranking system right now
There is a fairly basic ranking system when searching in the inserter. Exact matches have the highest ranking, partial matches where the title starts with the search term are next, and then individual word matches are ranked lowest. Finally, core blocks are given a small boost in the results.
There was a problem hiding this comment.
and the one we need would be different than the inserter, that uses frecency.
Sorry for not being clear here. I didn't expand on all the details about the other ranking system and only mentioned frecency. What I really meant to say is that we would need to also include a sorting system for patterns in every list/search results, which is the missing piece. For example in patterns we should have a way to rank better patterns from current theme, or external patterns loaded from theme.json etc..
|
Size Change: -99 B (0%) Total Size: 1.26 MB
ℹ️ View Unchanged
|
|
Nice! It's working well for me: Kapture.2022-07-15.at.14.16.01.mp4It's a shame the search input isn't sticky, but I don't suppose we can do that without updating the modal component? |
Correct. I will look into the sticky topbar while working on improvements in #39308. |
There was a problem hiding this comment.
This works nicely, and it's great to see how easy to follow the code is.
I did wonder if it might be good to debounce the filtering on the input. If you type quickly it gets a bit flickery. The inserter search doesn't do this though, so perhaps not a requirement for merging.
BTW - the block manager has sticky sections, so that could help with making the search bar sticky.
| export function searchItems( items, searchValue ) { | ||
| if ( ! searchValue ) { | ||
| return items; | ||
| } | ||
|
|
||
| const normalizedSearchValue = searchValue.toLowerCase(); | ||
| return items.filter( ( item ) => { | ||
| const normalizedTitle = item.title.toLowerCase(); | ||
|
|
||
| return normalizedTitle.includes( normalizedSearchValue ); | ||
| } ); |
There was a problem hiding this comment.
I think it's ok to start with something simple, but also looks like it probably wouldn't be too difficult to improve it. I had a look at the code for the inserter to see how it works: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/components/inserter/search-items.js#L145-L161
The biggest win would probably be to still consider it a match if all of the search term words are in the title but order doesn't matter (e.g. 'large dark' and 'dark large' produce the same search results), which seems to be what this bit of code does using the words function:
gutenberg/packages/block-editor/src/components/inserter/search-items.js
Lines 144 to 162 in dfc4b8b
It'll be a bit easier for patterns since there's only one title field to check against.
edit: Also just saw that lodash words is about to be replaced in #42467.
packages/block-library/src/template-part/edit/selection-modal.js
Outdated
Show resolved
Hide resolved
| export function searchItems( items, searchValue ) { | ||
| if ( ! searchValue ) { | ||
| return items; | ||
| } | ||
|
|
||
| const normalizedSearchValue = searchValue.toLowerCase(); | ||
| return items.filter( ( item ) => { | ||
| const normalizedTitle = item.title.toLowerCase(); | ||
|
|
||
| return normalizedTitle.includes( normalizedSearchValue ); | ||
| } ); |
There was a problem hiding this comment.
This is a very basic search. I'm not sure if we need a ranked search like inserter here.
We don't actually have a ranking system right now and the one we need would be different than the inserter, that uses frecency.
Having said that, I think we can start small with a couple of tweaks maybe:
- Rename the function to something more specific(ex.
searchPatterns) as is not generic as some other similar functions - Probably add simple
accents, trimhandling tonormalizedTitle.
In general I think integrating pattern explorer here would be fitting, but until we have something like that, it's okay to have at least a basic search by title. Also when we'll work on a patterns ranking system we might convert similar functions to a selector..
|
@jameskoster, I made the search component sticky. I'm unsure about the top padding, I would appreciate your feedback on this matter. |
| const rankedPatterns = patterns | ||
| .map( ( pattern ) => { | ||
| return [ pattern, getPatternSearchRank( pattern, searchValue ) ]; | ||
| } ) | ||
| .filter( ( [ , rank ] ) => rank > 0 ); | ||
|
|
||
| rankedPatterns.sort( ( [ , rank1 ], [ , rank2 ] ) => rank2 - rank1 ); | ||
| return rankedPatterns.map( ( [ pattern ] ) => pattern ); |
There was a problem hiding this comment.
I borrowed this from the searchItems function.
|
@Mamaduka there's some awkward tension between the search input and the results, and the border appearing beneath the header feels a bit strange too. We could address these issues but it would involve overriding the modal component behaviour which probably isn't ideal. Would it be considered too hacky to position the search absolutely so that it 'appears' in the header? search.mp4This would be a temporary measure until #39308. What do you think? |
I think it's a bit hacky, but let's give it a try 😄 |
|
I adjusted the search input height so that it is centrally aligned. For me this works a bit better than the full width version, but I appreciate the hacky-ness may not be acceptable 🙈 |
|
@talldan, I decided to skip debouncing for now. |
|
One potential problem with the absolutely positioned search input is that it could overlap the modal title on small screens. I've pushed a change to reduce the width, and it works well for English but will need testing in other languages:
It might be better if it only becomes absolutely positioned on larger screens. If the width is an issue for other languages I'll work on that. Edit: Pretty safe to assume it will be an issue. I'll work on making the input absolutely positioned only on large screens 😆 |
|
Thank you, @jameskoster. |
|
Pushed an update to switch the search position based on screen size: search.mp4 |
|
@jameskoster do you think the double I actually closed the modal in my testing now, but my intention was to clear the search.. |
|
Well I didn't think so, but now you got me 🤔 I suppose it's safer to remove the absolute positioning and address the modal layout in #39308. |
|
@talldan, @ntsekouras, I think this is ready for final review. |
There was a problem hiding this comment.
Nice work, code looks good to me.
I can see why it'd be better with some of the modal header styles adjusted, the search box gets a little lost next to the patterns, in some instances it almost feels like its part of a pattern:
I know there are some further iterations slated to happen.
BTW, in relation to the template part selection modal, I have refactored this in #42142 to be more generic, and I'll bring these changes into that PR. I'm not sure how #39308 will affect things though. Just thought I'd mention it in case this refactored version is useful to you in any future work.
|
Thank you, @talldan. |
What?
Resolves: #40791
Adds a new search functionality to the template parts replacement modal.
Why?
It makes it easier to find specific template parts.
How?
SearchControlcomponent to the modal.searchItemsto perform a basic search based on a string match.Testing Instructions
Screenshots or screencast
CleanShot.2022-07-15.at.12.17.42.mp4