feat: add experimental single question version #44
Conversation
| showGoal: true, | ||
| showCurrentJobTitle: true, | ||
| alwaysShowCareerInterest: false, | ||
| useProgressive: false, |
There was a problem hiding this comment.
The word "use" is often reserved for hooks so this feels like maybe it could be confusing to a new dev. Maybe isProgressive or showProgressive are better options?
There was a problem hiding this comment.
Good call. Changed to isProgressive.
|
|
||
| return ( | ||
|
|
||
| useInteractiveBoxSet ? interactiveBox() : selectableBox() |
There was a problem hiding this comment.
Is this necessary? I thought it would be covered by the conditional in ViewResults at line 154
{ useInteractiveBoxSet ? (
<RelatedSkillsInteractiveBoxSet
jobSkillsList={jobSkillsList}
selectedJobTitle={selectedJobTitle}
onChange={handleJobTitleChange}
useInteractiveBoxSet={useInteractiveBoxSet}
/>
) : (
<RelatedSkillsSelectableBoxSet
jobSkillsList={jobSkillsList}
selectedJobTitle={selectedJobTitle}
onChange={handleJobTitleChange}
/>
)}There was a problem hiding this comment.
It is not! I am in the process of factoring it out. I wasn't sure originally if I wanted one component or two.
There was a problem hiding this comment.
It was a remnant of a first pass. Factored out now.
| </Alert> | ||
| )} | ||
| { /* This should just pass the useInteractiveBoxSet flag to the component */ } | ||
| { useInteractiveBoxSet ? ( |
There was a problem hiding this comment.
Same as the useProgressive boolean. This would probably be better off written as isInteractiveBoxSet or showInteractiveBoxSet to avoid devs thinking it is a hook
There was a problem hiding this comment.
changed to isInteractiveBoxSet.
| const CardComponent = ({ name, skills }) => ( | ||
| <Card | ||
| isClickable | ||
| onClick={handleCareerInterestSelect} |
There was a problem hiding this comment.
This doesn't appear to be doing anything when I test in my local environment. This function handleCareerInterestSelect is used add to the careerInterests array which doesn't change the what the recommendations are in the UI.
I think we need to utilize the onChange function here which calls handleJobTitleChange from ViewResults. Unfortunately, this function is expecting the clicked item to be a control with a e.target.value but since it's a card, we'll have to find a different way to pass the value. This value should be the name of the job so that the rest of the function works properly.
Or perhaps we need an entirely new function for this?
There was a problem hiding this comment.
Correct. This part doesn't actually work. I would love to get your input on how to implement this correctly.
There was a problem hiding this comment.
Yes, we need an entirely new function for this.
There was a problem hiding this comment.
Approved 👍 after talking in meeting about a follow up ticket
Codecov Report
@@ Coverage Diff @@
## main #44 +/- ##
==========================================
- Coverage 91.59% 85.12% -6.48%
==========================================
Files 30 32 +2
Lines 369 457 +88
Branches 68 90 +22
==========================================
+ Hits 338 389 +51
- Misses 30 65 +35
- Partials 1 3 +2
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Add a query parameter "viz=1q" that eliminates the multiple questions and the next button and goes straight to recommendations after asking the question about the job title.
The default behavior without the query parameter is unchanged.