Skip to content

[QUIZ-815] - Facets as options#234

Merged
TarekAlQaddy merged 9 commits intoConstructor-io:mainfrom
vladconstructor:QUIZ-815/quiz-facets-as-options-customer-library
Jul 17, 2025
Merged

[QUIZ-815] - Facets as options#234
TarekAlQaddy merged 9 commits intoConstructor-io:mainfrom
vladconstructor:QUIZ-815/quiz-facets-as-options-customer-library

Conversation

@vladconstructor
Copy link
Copy Markdown
Contributor

@vladconstructor vladconstructor commented Jun 4, 2025

Was blocked by Constructor-io/constructorio-client-javascript#379

Introducing a new facet-based question type to ui-library.

@vladconstructor vladconstructor requested a review from a team as a code owner June 4, 2025 13:19
Copy link
Copy Markdown
Contributor

@v-i-s-h-n-u-ps v-i-s-h-n-u-ps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vladconstructor Changes look good. Just a couple of nitpicks. Rest will leave it to customer integrations

Comment thread src/hooks/usePropsGetters/useSelectInputProps.ts Outdated
Comment on lines +110 to +112
(currentQuestionData?.type === 'single' ||
currentQuestionData?.type === 'single_filter_value') &&
singleSelectClicked.current
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And here

Copy link
Copy Markdown
Contributor

@madeel20 madeel20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Comment thread src/hooks/usePropsGetters/useSelectInputProps.ts Outdated
…zzes into QUIZ-815/quiz-facets-as-options-customer-library
Copy link
Copy Markdown
Contributor

@TarekAlQaddy TarekAlQaddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after making sure that all tests are passing locally

@TarekAlQaddy TarekAlQaddy merged commit 0d545f1 into Constructor-io:main Jul 17, 2025
8 of 9 checks passed
singleSelectClicked.current = true;
setSelected({ [id]: true });
} else if (type === QuestionTypes.MultipleSelect) {
return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why return?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to avoid additional checks

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also put switch/case statement, but felt unnecessary for only 2 conditions

Comment thread .d.ts
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did we delete this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an empty file
Do we need it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants