CS-10802: Refactor SearchPanel – extract utilities and simplify args#4418
CS-10802: Refactor SearchPanel – extract utilities and simplify args#4418
Conversation
…cerns Replace cluttered args (consumingRealm, preselectConsumingRealm, availableRealmUrls, onFilterChange) with cleaner domain-typed alternatives (initialSelectedRealms, initialSelectedTypes, onRealmChange, onTypeChange) so callers deal with URL[] and ResolvedCodeRef[] instead of PickerOption[] and booleans. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace SearchPanel's internal-leaking args with domain types (URL[], ResolvedCodeRef[] instead of PickerOption[], booleans) - Extract TypeSummariesResource from SearchPanel - Introduce RealmFilter/TypeFilter interfaces to bundle picker args - Move utilities to app/utils/card-search/ (7 focused files) - Deduplicate URL parsing into shared isURLSearchKey/resolveSearchKeyAsURL - Extract section building, pagination, type filtering, recent cards filtering into pure helper functions - Slim SearchContent from 736 to ~500 lines Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Preview deployments |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf6d5d0ea6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!result) { | ||
| this._isLoading = false; | ||
| return; |
There was a problem hiding this comment.
If fetchCardTypeSummaries returns undefined (network error, auth/login race, or canFetch false), this branch exits without resetting _typeSummariesData/_options/_selected. After a realm change, the picker can continue showing types from the previous realm and keep an invalid selection applied to subsequent queries, which is a behavior regression from the previous implementation that cleared state on fetch failure.
Useful? React with 👍 / 👎.
| if (realmURLsKey !== prevKey) { | ||
| this.#previousRealmURLs = realmURLs; | ||
| this._typeSearchKey = ''; | ||
| this.fetchTypeSummariesTask.perform(realmURLs, ''); | ||
| } |
There was a problem hiding this comment.
modify() stores new baseFilter/initialSelectedTypes on every update, but it only triggers a recompute path when realmURLs changes. If SearchSheet calls doExternallyTriggeredSearch() while the sheet is already open (same realms, new typeRef), the new initial type is ignored and the previous type selection remains active, so the externally requested type-constrained search does not take effect.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Refactors the card search UI by simplifying SearchPanel’s public API to domain types, extracting type-summary fetching into a dedicated resource, and moving search logic into focused utilities.
Changes:
- Replaced
SearchPanelpicker/boolean args with domain-levelURL[]/ResolvedCodeRef[]andRealmFilter/TypeFilterinterfaces. - Extracted type summary fetching/pagination into
TypeSummariesResource, and moved search helpers (URL parsing, type filtering, section building, recents logic) intoapp/utils/card-search/*. - Updated host/operator-mode UI + Boxel Picker to display checkbox selection state and adjusted integration tests accordingly.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/host/tests/integration/components/operator-mode-ui-test.gts | Updates integration assertions to use picker checkbox-selected UI + new TypeSummariesResource.PAGE_SIZE. |
| packages/host/app/utils/card-search/url.ts | Adds shared URL/empty-search helpers for search key parsing. |
| packages/host/app/utils/card-search/types.ts | Introduces domain types (NewCardArgs) and shared removeFileExtension. |
| packages/host/app/utils/card-search/type-filter.ts | Extracts filter/type-ref logic (root type handling, subtype checks, type-ref filtering). |
| packages/host/app/utils/card-search/sections.ts | Extracts section data types + builders (recents/url/query sections) and assembly ordering. |
| packages/host/app/utils/card-search/section-pagination.ts | Extracts section focus + “show more” pagination into a tracked helper class. |
| packages/host/app/utils/card-search/recent-cards.ts | Extracts recent-card filtering + sorting logic (including type constraints). |
| packages/host/app/utils/card-search/query-builder.ts | Removes duplicated helpers and reuses new URL/empty helpers; keeps query construction centralized. |
| packages/host/app/services/realm-server.ts | Adds canFetch guard used by type summaries resource to avoid early fetch attempts. |
| packages/host/app/services/operator-mode-state-service.ts | Repoints removeFileExtension import to new utils location. |
| packages/host/app/resources/type-summaries.ts | New resource for fetching/paginating type summaries and maintaining selected types. |
| packages/host/app/components/type-picker/index.gts | Switches to TypeFilter interface and maps domain options/selection to Picker options. |
| packages/host/app/components/search-sheet/index.gts | Uses shared URL helpers and new SearchPanel args/events (onRealmChange/onTypeChange). |
| packages/host/app/components/realm-picker/index.gts | Switches to RealmFilter interface and maps selected URLs to Picker options. |
| packages/host/app/components/operator-mode/operator-mode-overlays.gts | Repoints removeFileExtension import to new utils location. |
| packages/host/app/components/card-search/section-header.gts | Imports RealmSectionInfo from extracted sections util types. |
| packages/host/app/components/card-search/search-result-section.gts | Imports section + NewCardArgs types from extracted utils. |
| packages/host/app/components/card-search/search-result-header.gts | Imports NewCardArgs from extracted utils. |
| packages/host/app/components/card-search/search-content.gts | Large refactor: uses extracted helpers/resources; replaces internal pagination/section building. |
| packages/host/app/components/card-search/search-bar.gts | Simplifies args to accept realmFilter / typeFilter. |
| packages/host/app/components/card-search/panel.gts | Major simplification: delegates type summaries to resource and yields domain filter objects. |
| packages/host/app/components/card-search/item-button.gts | Repoints removeFileExtension/NewCardArgs imports to new utils. |
| packages/host/app/components/card-search/constants.ts | Adds clarification about dual-mode sorting + recent-card sorting helper. |
| packages/host/app/components/card-catalog/modal.gts | Updates SearchPanel usage for new args, and derives initial selected realms. |
| packages/boxel-ui/test-app/tests/integration/components/picker-test.gts | Adds coverage for checkbox selected-state rendering in picker dropdown. |
| packages/boxel-ui/addon/src/components/picker/index.gts | Changes selected-state logic to use id equality instead of object identity. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return selected.map((url) => { | ||
| let realmURL = url.href; | ||
| let info = this.realm.info(realmURL); | ||
| let label = info?.name ?? this.realmDisplayNameFromURL(realmURL); | ||
| let icon = info?.iconURL ?? undefined; | ||
| return { id: realmURL, icon, label, type: 'option' as const }; | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
pickerSelected builds new PickerOption objects instead of reusing the option instances from realmOptions. Because the underlying PowerSelect selection logic relies on object identity, this can break highlighting/toggling (e.g., clicking a selected realm may add a duplicate instead of deselecting, and selected options may not be recognized by PowerSelect). Build @selected by looking up the corresponding objects from the same @options array (and consider caching realmOptions so @options/@selected share stable references).
| return selected.map((url) => { | |
| let realmURL = url.href; | |
| let info = this.realm.info(realmURL); | |
| let label = info?.name ?? this.realmDisplayNameFromURL(realmURL); | |
| let icon = info?.iconURL ?? undefined; | |
| return { id: realmURL, icon, label, type: 'option' as const }; | |
| }); | |
| } | |
| let optionsById = new Map( | |
| this.realmOptions.map((option) => [option.id, option] as const), | |
| ); | |
| return selected.flatMap((url) => { | |
| let option = optionsById.get(url.href); | |
| return option ? [option] : []; | |
| }); | |
| } | |
| @cached |
| private get pickerOptions(): PickerOption[] { | ||
| const options: PickerOption[] = this.args.filter.options.map((opt) => ({ | ||
| id: opt.id, | ||
| label: opt.displayName, | ||
| tooltip: opt.id, | ||
| icon: opt.icon, | ||
| type: 'option' as const, | ||
| })); | ||
| return [this.selectAllOption, ...options]; | ||
| } | ||
|
|
||
| get selected(): PickerOption[] { | ||
| return this.args.selected.length > 0 | ||
| ? this.args.selected | ||
| : [this.selectAllOption]; | ||
| private get pickerSelected(): PickerOption[] { | ||
| const selectedKeys = new Set( | ||
| this.args.filter.selected.map((ref) => internalKeyFor(ref, undefined)), | ||
| ); | ||
| if (selectedKeys.size === 0) { | ||
| return [this.selectAllOption]; | ||
| } | ||
| return this.pickerOptions.filter( | ||
| (opt) => opt.type !== 'select-all' && selectedKeys.has(opt.id), | ||
| ); | ||
| } |
There was a problem hiding this comment.
pickerOptions and pickerSelected each compute fresh PickerOption object arrays. In the template, @options={{this.pickerOptions}} and @selected={{this.pickerSelected}} will therefore often contain different object instances for the same ids, which can break PowerSelect’s identity-based selection/toggle behavior (selected items may not deselect, duplicates can appear). Cache pickerOptions (e.g., via @cached) and derive pickerSelected by selecting references from that cached array (or otherwise ensure @selected is composed of the exact objects found in @options).
|
|
||
| this.#baseFilter = baseFilter; | ||
| this.#initialSelectedTypes = initialSelectedTypes; | ||
|
|
||
| // Only re-fetch when realmURLs change (search key changes are handled by onSearchChange) | ||
| let realmURLsKey = realmURLs.join(','); | ||
| let prevKey = this.#previousRealmURLs?.join(','); | ||
|
|
||
| if (realmURLsKey !== prevKey) { | ||
| this.#previousRealmURLs = realmURLs; | ||
| this._typeSearchKey = ''; | ||
| this.fetchTypeSummariesTask.perform(realmURLs, ''); |
There was a problem hiding this comment.
modify() updates #baseFilter / #initialSelectedTypes but only triggers work when realmURLs changes. That means changes to baseFilter (and potentially initialSelectedTypes) won’t update _options / _selected until the next realm change/search fetch, which can leave the UI showing stale type options while hasNonRootBaseFilter has already flipped. Consider calling recomputeTypeFilter() when these inputs change (and/or tracking previous baseFilter/initialSelectedTypes so you can recompute without refetching).
| this.#baseFilter = baseFilter; | |
| this.#initialSelectedTypes = initialSelectedTypes; | |
| // Only re-fetch when realmURLs change (search key changes are handled by onSearchChange) | |
| let realmURLsKey = realmURLs.join(','); | |
| let prevKey = this.#previousRealmURLs?.join(','); | |
| if (realmURLsKey !== prevKey) { | |
| this.#previousRealmURLs = realmURLs; | |
| this._typeSearchKey = ''; | |
| this.fetchTypeSummariesTask.perform(realmURLs, ''); | |
| let previousBaseFilter = this.#baseFilter; | |
| let previousInitialSelectedTypes = this.#initialSelectedTypes; | |
| this.#baseFilter = baseFilter; | |
| this.#initialSelectedTypes = initialSelectedTypes; | |
| // Only re-fetch when realmURLs change (search key changes are handled by onSearchChange) | |
| let realmURLsKey = realmURLs.join(','); | |
| let prevKey = this.#previousRealmURLs?.join(','); | |
| let didBaseFilterChange = previousBaseFilter !== baseFilter; | |
| let didInitialSelectedTypesChange = | |
| previousInitialSelectedTypes !== initialSelectedTypes; | |
| if (realmURLsKey !== prevKey) { | |
| this.#previousRealmURLs = realmURLs; | |
| this._typeSearchKey = ''; | |
| this.fetchTypeSummariesTask.perform(realmURLs, ''); | |
| } else if (didBaseFilterChange || didInitialSelectedTypesChange) { | |
| this.recomputeTypeFilter(); |
Summary
URL[],ResolvedCodeRef[]) instead of internal concerns (PickerOption[], booleans)RealmFilter/TypeFilterinterfacesapp/utils/card-search/(7 focused files: url, query-builder, type-filter, recent-cards, sections, section-pagination, types)isURLSearchKey()/resolveSearchKeyAsURL()Test plan
Linear: https://linear.app/cardstack/issue/CS-10802
🤖 Generated with Claude Code