-
Notifications
You must be signed in to change notification settings - Fork 4
No ref/new prop to jump to question #251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
No ref/new prop to jump to question #251
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a new "jump to question" feature for the quiz component, allowing users to navigate directly to a specific question within the quiz flow.
- Adds a new
jumpToQuestionevent handler and supporting types - Implements jump functionality that restricts navigation to previously answered questions
- Updates documentation to correct the
quizAnswerChangedevent description
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types.ts | Adds type definitions for JumpToQuestion functionality and related prop getters |
| src/stories/Quiz/Hooks/Docs/markdown/EventsDocs.md | Corrects documentation for quizAnswerChanged event |
| src/hooks/useQuizEvents/useQuizResetClick.ts | Fixes return type from NextQuestion to ResetQuiz |
| src/hooks/useQuizEvents/useJumpToQuestion.ts | Implements jump to question logic with validation |
| src/hooks/useQuizEvents/index.ts | Integrates jump to question hook into main events |
| src/hooks/usePropsGetters/useJumpToQuestionButtonProps.ts | Provides button props for jump functionality |
| src/hooks/usePropsGetters/index.ts | Exports jump to question button props getter |
| src/components/CioQuiz/quizLocalReducer.ts | Handles jump to question state management and refactors answer input handling |
| src/components/CioQuiz/quizApiReducer.ts | Clears quiz results when jumping to question |
| src/components/CioQuiz/index.tsx | Exposes jump to question props to component |
| src/components/CioQuiz/context.ts | Adds jump to question to context interface |
| src/components/CioQuiz/actions.ts | Defines jump to question action types |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| tabIndex: buttonDisabled ? -1 : 0, | ||
| 'aria-disabled': buttonDisabled ? 'true' : 'false', | ||
| 'aria-describedby': buttonDisabled ? 'jump-to-button-help' : '', | ||
| type: 'button', |
Copilot
AI
Sep 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return object includes accessibility attributes (tabIndex, aria-disabled, aria-describedby, type) but the JumpToQuestionButtonProps interface only defines className, onClick, and style. Either update the interface to include these properties or remove them from the return object.
TarekAlQaddy
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for making the changes @v-i-s-h-n-u-ps . I’ve left a few comments for your review. Would it also be possible to include tests for the new hooks? It would be great if the new hook could be added to the hooks documentation as well.
|
Tests and documentations |
|
@v-i-s-h-n-u-ps Thanks for working on the comments! I tested the logic of jumping back to a question and found 2 issues:
Here's a proposed solution for these 2 issues, could you please review them and apply the changes here in the PR if all is good |
|
Thank you for the review and finding that issue. I also added some tests to confirm the behaviour. |
| currentQuestionData?.type === QuestionTypes.SingleFilterValue) && | ||
| singleSelectClicked.current | ||
| singleSelectClicked.current && | ||
| nextQuestionOnSingleSelect |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you clarify what's the use case of this additional option? Since it's a single select question it should move forward when a single value has been selected. Why should we give the option of stopping that logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was an ask from one of our customer. And I thought it made sense since we would not want accidental clicks to go to next question as soon as you select an option.
|
Merged in #254 |
No description provided.