Category search modal icon update #13092
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Thanks @marcellamaki! Looks good! Just a little observation :).
Also there are some failing tests, this is because of the KDS upgrade, specifically because of the KSelect dropdown updates that now renders the dropdown as a popper, and the tests in GenderSelect are incorrectly assuming that the dropdown options are in the DOM just after rendering the component, which isnt true anymore (in the past they were, but they werent visible), a simple await userEvent.click(screen.getByText('Gender')); before trying to query the options should make it work.
| // for those with a clearer 1:1 match with the category and icon | ||
| else { | ||
| const categoryWordsString = category.toLowerCase().split('_'); | ||
| for (let i = 1; i < categoryWordsString.length; i++) { |
There was a problem hiding this comment.
With this we are transforming snake case to camel case here, right? I see that above in this file we use the lodash/camelCase function to transform some other snake case strings, I think we can also use this here.
There was a problem hiding this comment.
ah, that would be a great simplification! thanks Alex
Yes, good point. I wonder if it would make more sense for me to open a separate PR to do the upgrade, and resolve that, and then open this separately? |
|
Yes! Sounds good :) |
Summary
After:
References
Fixes #13042
Reviewer guidance
Navigate to the lessonstemp path in coach and begin a search. All icons should display, and should match the figma spec.