[CSL-2867] Section configuration option updates #112
Conversation
src/components/Autocomplete/SectionItemsList/SectionItemsList.tsx
Outdated
Show resolved
Hide resolved
src/components/Autocomplete/SectionItemsList/SectionItemsList.tsx
Outdated
Show resolved
Hide resolved
…ation-option-updates
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
@esezen This will need to be updated based on the introduced changes. It's using Also this constructorio-ui-autocomplete/src/utils.ts Line 196 in b3305af |
|
I believe we need to add a type guard here. I believe this should work constructorio-ui-autocomplete/src/utils.ts Line 196 in b3305af 
|
| export interface CustomSection extends CustomSectionConfiguration { | ||
| data: Item[]; | ||
| } | ||
|
|
There was a problem hiding this comment.
The above is becoming confusing, redundant and introduces unnecessary types that makes it hard to understand the different configs 😭
What do you think of adding a utility type
type WithData<T> = T & { data: Item[] };export type Section = WithData<AutocompleteSectionConfiguration> | WithData<RecommendationsSectionConfiguration> | WithData<CustomSectionConfiguration>;
This will trim down types especially that they're not used out of this file
There was a problem hiding this comment.
I'm not against this change but the code you posted seems more confusing than what we have. It's shorter but it's more confusing. Let's talk about this today
There was a problem hiding this comment.
Leave the names alone but add comments
src/hooks/useCioAutocomplete.ts
Outdated
|
|
||
| if (type === 'autocomplete' || !type) { | ||
| if (config.identifier && !config.indexSection) { | ||
| return { ...config, indexSection: config.identifier }; |
There was a problem hiding this comment.
Maybe we can add type: 'autocomplete' here when !type instead of making it required
Context https://github.com/Constructor-io/constructorio-ui-autocomplete/pull/112/files#r1436618676
There was a problem hiding this comment.
We can, I'm just not entirely sure how we would type it. Let's pair on it
esezen
deleted the
csl-2867-os-ui-autocomplete-section-configuration-option-updates
branch
No description provided.