add homescreen search filters for categorizable things rather than searchable terms - #11618
Joey Wunderlich (jwunderl) wants to merge 8 commits into
Conversation
|
Copilot add support for a code card fields that specifies and override the inferred type / language values per card (so that e.g. we can correctly tag videos with blocks or typescript,etc). Should be implemented very similar to how the other 3 filter types are |
Co-authored-by: jwunderl <5615930+jwunderl@users.noreply.github.com>
Implemented in |
|
Copilot resolve the merge conflicts in this pull request |
…-filters # Conflicts: # cli/cli.ts # pxtlib/gallery.ts # webapp/src/projects.tsx Co-authored-by: jwunderl <5615930+jwunderl@users.noreply.github.com>
Resolved and pushed in merge commit |
Hassan Sufi (aznhassan)
left a comment
There was a problem hiding this comment.
Looks good! Nice to have expanded filter functionality on the home page!
| } | ||
|
|
||
| const activityTypeOptions: HomeSearchFilterOption[] = [ | ||
| { id: "tutorial", label: lf("Tutorials") }, |
There was a problem hiding this comment.
it's not safe to call lf top-level in a module because the localizations might not have been loaded at the time of import. i'd create a function to return these constants
|
|
||
| function getActionEditor(cardType: pxt.CodeCardType, editor?: pxt.CodeCardEditorType): pxt.CodeCardEditorType { | ||
| if (editor) return editor; | ||
| if (cardType === "tutorial" || cardType === "example") return "blocks"; |
There was a problem hiding this comment.
this seems inaccurate... we can have ts only and python only tutorials and examples
There was a problem hiding this comment.
will remove yeah, i added support for overriding the 'predetermined' categories with card fields as a last step to support youtube videos but better to just manually assign those here rather than risk being wrong
| const allSearchCards = searchMode | ||
| ? this.collectSearchEntries(this.getSearchGalleries()).cards | ||
| : []; | ||
| const availableSearchFilters = getAvailableHomeSearchFilters(allSearchCards); |
There was a problem hiding this comment.
should this be memoized or something? seems expensive
| const searchFilterCandidates = searchQuery.trim() | ||
| ? this.state.searchCandidates || [] | ||
| : allSearchCards; | ||
| const searchFilterOptionCounts = getHomeSearchFilterOptionCounts( |
There was a problem hiding this comment.
again, seems like this should be memoized. it's a shame this isn't a function component, then you could just use React.useMemo()
| icon="search icon" | ||
| ariaLabel={lf("Search tutorials, examples, and projects")} | ||
| /> | ||
| {!!availableSearchFilters.length && <div className="home-search-filters" role="group" aria-label={lf("Filter search results")}> |
There was a problem hiding this comment.
is tabbing between filter dropdowns the correct option here or should this list be navigated by arrow keys? this is a common design pattern, let's make sure we're doing the keyboard navigation right
There was a problem hiding this comment.
tab between dropdowns / arrow key through is what i thought to be standard but will try and find some more ones like this; i know i've seen this design in plenty of places before but the ones that came to mind to check all seem to have switched to the be to be long lists of checkboxes to the side instead which is (e.g. like amazon search results), need to look around more
…e, and more noticeable highlight
…osoft/pxt into dev/jwunderl/search-filters Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
build with those changes https://arcade.makecode.com/app/875c182baf3e0df3742823ea1edb1f600534b055-52e0723312 -- it was snappy enough to not be noticeable up to at least 6x cpu throttle, but 20x cpu throttle had it as ~maybe 200ms filter before and snappier now depending on cache. also fixed some accessibility bits (highlighting, not snapping closed immediately when first selection entered) closest to a direct match could find for behavior was https://www.google.com/travel/flights/search (filters show up after you put in any search, can just plug in e.g. bc and random dates), and it is tab between menus (those menus more complicated than checkboxes though / different controls per menu) |
was thinking and this is something we said we would do but might push off for a bit -- but packaging into next release would probably be nice, to have it as more of a useful thing. had copilot do a first pass summarizing for both arcade and minecraft and will put those up, the fields added are new so would be fine to merge.
build at bottom only shows 2, but when cards define the other fields they'll pop in:
https://arcade.makecode.com/app/4cbea7fd57f3158a24dc0b9426ae59a7cf9bd69d-7b9370bf7f