[CDX-310]: added recent searches to autocomplete dropdown by niizom · Pull Request #267 · Constructor-io/constructorio-ui-autocomplete

niizom · 2026-03-13T19:32:15Z

No description provided.

esezen · 2026-04-24T19:07:57Z


-export type Section = AutocompleteSection | RecommendationsSection | CustomSection;
+export interface RecentSearchesSection extends RecentSearchesSectionConfiguration {
+  data: Item[];


I think this can be

Suggested change

data: Item[];

data: RecentSearch[];

wdyt @niizom ?

Narrowing down this type will require a code refactor which might be time consuming

esezen

Thanks for working on this!

esezen · 2026-05-14T21:36:20Z

I think there is a bug here and also affects useRemoveSections hook. When the variant custom_autosuggest_ui_disable_recommendations_in_zero_state is active, it wipes any recentSearches section a customer has configured. A customer who only uses recentSearches (no recommendations) sees nothing under that A/B variant. The same applies to the opening the menu logic here

esezen · 2026-05-14T21:43:40Z

-     * Transforms a `SearchSuggestion` into the desired URL string to be used when rendering anchor tags
+     * Transforms a `SearchSuggestion` or `RecentSearch` into the desired URL string
+     *  to be used when rendering anchor tags
     * i.e. <a href=getSearchResultsUrl([selected_search_suggestion])>[Search Suggestion]</a>


Can we update this line as well?

esezen · 2026-05-14T21:44:45Z

+  isInGroupSuggestion,
+  isRecentSearch,
+  isRecommendationsSection,
+  isSearchSuggestion,


esezen · 2026-05-14T21:49:12Z


-export type Section = AutocompleteSection | RecommendationsSection | CustomSection;
+export interface RecentSearchesSection extends RecentSearchesSectionConfiguration {
+  data: Item[];


Suggested change

data: Item[];

data: RecentSearch[];

esezen · 2026-05-14T21:50:54Z

+export type RecentSearch = Partial<ItemBase> &
+  Pick<ItemBase, 'value' | 'id'> & {
+    section: 'recent-searches';
+    ts: number;
+    data?: Record<string, unknown>;
+  };


I think we can simplify and make this tighter. Wdyt?

Suggested change

export type RecentSearch = Partial<ItemBase> &

Pick<ItemBase, 'value' | 'id'> & {

section: 'recent-searches';

ts: number;

data?: Record<string, unknown>;

};

export type RecentSearch = {

section: 'recent-searches';

value: string;

id: string;

ts: number;

data?: Record<string, unknown>;

};

Also, I might be missing something by why do we use recentSearches in certain places and recent-searches in others?

esezen · 2026-05-14T21:53:42Z

+      recentSearchesResults[displayName] = recentSearchesFromStore
+        .slice()
+        .reverse()
+        .slice(0, numResults || DEFAULT_NUM_RESULTS)


Can we document the default that we use here in docs and / or in types?

esezen · 2026-05-14T21:59:28Z

+        .map(({ term, ts, data }) => ({
+          ts,
+          data,
+          id: `${term}-${ts}`,


Why do we add ts here?

constructor-claude-bedrock

Code Review

This PR adds a recentSearches section type to the autocomplete dropdown zero state, reading from localStorage and integrating with tracking — the implementation is well-structured overall but has a dead-code bug and several type-safety/mutation concerns worth addressing before merge.

Inline comments: 7 discussions added

Overall Assessment: ⚠️ Needs Work

constructor-claude-bedrock · 2026-06-01T21:03:01Z

              trackAutocompleteSelect(cioClient, selectedItem.value, selectData);
+
+              // Track recent searches as Search Suggestions
+            } else if (selectedItem.section === 'recent-searches') {


Critical Issue: This else if branch is dead code and will never execute. The outer if at line 41–44 already checks selectedItem?.section === 'recent-searches' and runs trackSearchSubmit + setQuery when true, so control never falls through to this branch. This means trackAutocompleteSelect is never called for recent searches, silently dropping the select-tracking event.

Fix: Move the trackAutocompleteSelect call for recent-searches inside the first if block (alongside trackSearchSubmit), or restructure so both calls execute:

if ( selectedItem?.section === 'Search Suggestions' || selectedItem?.section === 'recent-searches' ) { setQuery(selectedItem.value || ''); trackSearchSubmit(cioClient, selectedItem.value, { originalQuery: previousQuery }); if (selectedItem.section === 'recent-searches') { trackAutocompleteSelect(cioClient, selectedItem.value, { originalQuery: previousQuery, section: 'Search Suggestions', }); } }

constructor-claude-bedrock · 2026-06-01T21:03:02Z

+    });
+
+    setRecentSearches(recentSearchesResults);
+  }, [recentSearchesSections]);


Important Issue: recentSearchesSections is an array created fresh on every render in useSectionsResults (activeSections.filter(...)), so this dependency will always be a new reference, causing the effect to re-run on every render even when the section config hasn't changed. This creates a performance issue and unnecessary state updates that can cascade into re-renders.

Fix: Memoize activeRecentSearchesSections in useSectionsResults (already done — good), but make sure the upstream activeSections array itself is stable. Alternatively, stabilize the dependency with a deep comparison (e.g., useDeepCompareMemo) or serialize the relevant fields as a primitive dependency:

}, [JSON.stringify(recentSearchesSections)]);

constructor-claude-bedrock · 2026-06-01T21:03:02Z

-          openOnFocus !== false &&
-          features.featureDisplayZeroStateRecommendations
-        ) {
+        if (zeroStateActiveSections && openOnFocus !== false && activeSections.length > 0) {


Important Issue: The previous condition features.featureDisplayZeroStateRecommendations was a deliberate feature flag guard — removing it means the zero state now always opens on focus regardless of this feature flag. If the flag is intended to control all zero state display (not just recommendations), this behavioral change is a regression. If the intent is that recent searches + custom sections should always open (bypassing the flag), the change is correct but should be explicitly documented in a comment.

constructor-claude-bedrock · 2026-06-01T21:03:02Z

 * Stores a recent search
 */
-export const storeRecentSearch = (term, suggestionData) => {
+export const storeRecentSearch = (term: string, suggestionData) => {


Important Issue: suggestionData remains untyped (any implicitly). Given that StoreRecentSearch was just introduced, the parameter should be typed — at minimum Record<string, unknown> or a dedicated type — to keep the function consistent with the rest of the typed additions in this PR.

constructor-claude-bedrock · 2026-06-01T21:03:02Z

+    const recentSearchesResults: SectionsData = {};
+
+    recentSearchesSections.forEach(({ displayName, numResults }) => {
+      recentSearchesResults[displayName] = recentSearchesFromStore


Suggestion: When two recentSearches sections share the same displayName (a user misconfiguration), the second will silently overwrite the first. Consider either (a) deduplicating by displayName with a warning, or (b) keying by a composite key to prevent silent data loss.

constructor-claude-bedrock · 2026-06-01T21:03:02Z

+  { term: 'blue jeans', ts: Date.now() - 2000 },
+  { term: 'white sneakers', ts: Date.now() - 1000 },
+];
+localStorage.setItem(recentSearchesStorageKey, JSON.stringify(mockRecentSearches));


Suggestion: The localStorage.setItem calls at module-level (lines ~590–595, outside any story play function) will run at import time and pollute localStorage for all stories in this file, not just the recent-searches ones. This can cause cross-story test interference. Move the setup exclusively inside the play functions (which is already done redundantly there), or use a beforeEach/loaders hook instead.

niizom requested review from Mudaafi and esezen as code owners March 13, 2026 19:32

Copilot AI review requested due to automatic review settings March 13, 2026 19:32

Copilot started reviewing on behalf of niizom March 13, 2026 19:32 View session

This comment was marked as outdated.

Sign in to view

niizom changed the title ~~[CDX-310]: added recent searches to autocomplete dropdown~~ DRAFT: [CDX-310]: added recent searches to autocomplete dropdown Mar 16, 2026

esezen requested a review from a team March 16, 2026 15:28

This comment was marked as outdated.

Sign in to view

TarekAlQaddy changed the title ~~DRAFT: [CDX-310]: added recent searches to autocomplete dropdown~~ [CDX-310]: added recent searches to autocomplete dropdown Apr 6, 2026

This comment was marked as outdated.

Sign in to view

TarekAlQaddy previously approved these changes Apr 6, 2026

View reviewed changes

esezen dismissed TarekAlQaddy’s stale review via b7c5220 April 24, 2026 18:45

esezen requested a review from a team as a code owner April 24, 2026 18:45

This comment was marked as outdated.

Sign in to view

esezen reviewed Apr 24, 2026

View reviewed changes

esezen requested changes May 14, 2026

View reviewed changes

[CDX-310]: added recent searches to autocomplete dropdown

53d9775

This comment was marked as outdated.

Sign in to view

Tarek Ahmed added 4 commits June 1, 2026 20:30

fixes, tracking, types, some adjustments

9e1132e

lint, address comments

a9bc3b3

Add tests and Storybook examples

374c756

Change default numResults in docs

6567e92

niizom force-pushed the cdx-310-ac-ui-feature-recently-searched-terms branch from 7ea7df2 to f4bbcdd Compare June 1, 2026 15:30

This comment was marked as outdated.

Sign in to view

[CDX-310]: fix PR review comments

8e4944c

niizom force-pushed the cdx-310-ac-ui-feature-recently-searched-terms branch from f4bbcdd to 8e4944c Compare June 1, 2026 21:01

constructor-claude-bedrock Bot reviewed Jun 1, 2026

View reviewed changes

Conversation

niizom commented Mar 13, 2026

Uh oh!

This comment was marked as outdated.

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

esezen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

constructor-claude-bedrock Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants