fix(theme): use correct tokens for the breadcrumbs #88
Conversation
|
|
|
| --tk-elements-breadcrumbs-dropdown-accordionIconColorHover: var(--tk-text-primary); | ||
| --tk-elements-breadcrumbs-dropdown-lessonBackgroundColor: var(--tk-elements-breadcrumbs-dropdown-backgroundColor); | ||
| --tk-elements-breadcrumbs-dropdown-lessonBackgroundColorSelected: var(--tk-background-secondary); | ||
| --tk-elements-breadcrumbs-dropdown-lessonTextColor: var(--tk-elements-breadcrumbs-dropdown-textColor); | ||
| --tk-elements-breadcrumbs-dropdown-lessonTextColorHover: var(--tk-elements-breadcrumbs-dropdon-textColorHover); | ||
| --tk-elements-breadcrumbs-dropdown-lessonTextColorSelected: var(--tk-elements-breadcrumbs-dropdown-lessonTextColor); | ||
| --tk-elements-breadcrumbs-dropdown-lessonTextColorHover: var(--tk-elements-breadcrumbs-dropdown-textColorHover); |
There was a problem hiding this comment.
This had a typo dropdon instead of dropdown.
| ></span> | ||
| <span className={navStyles.AccordionTriggerText}>{`Part ${partIndex + 1}: ${part.title}`}</span> | ||
| <span className={`${navStyles.AccordionTriggerIcon} i-ph-caret-right-bold scale-80`}></span> | ||
| <span>{`Part ${partIndex + 1}: ${part.title}`}</span> |
There was a problem hiding this comment.
Oh I hadn't realised that we injected Part <index> in the title. This is not ideal when writing a Tutorial in a language other than English.
I feel like we should remove it but not in this PR though.
Curious to hear your thoughts?
There was a problem hiding this comment.
OH you're right! We should do this differently.
There was a problem hiding this comment.
Do you know why we have those classes? It's the first time I'm noticing this CSS module. 👀
There was a problem hiding this comment.
At first I was moving all the UNO-css things inline. But then I realized it was annoying because that meant I had to copy and paste that every time the accordion trigger component is used.
So the classes are there (I think), so that you can just set navStyles.AccordionTrigger on the trigger, and the correct CSS is applied and you don't have to repeat the uno-css styles.
There was a problem hiding this comment.
Ah I see. I would argue we should have "presentational" components though to avoid the repetition.
We can always revisit that later 👍
|
@SamVerschueren Yep, I'll put them in Figma, thank you! |
When documenting the theming tokens, I noticed that some things didn't work correctly in regards the dropdown.
I also added 3 new tokens because I felt like it should be possible to style these things as well.
--tk-elements-breadcrumbs-dropdown-accordionTextColorSelected: The text color of the selected chapter/part in the dropdown.--tk-elements-breadcrumbs-dropdown-accordionIconColorSelected: The icon color of the selected chapter/part in the dropdown.--tk-elements-breadcrumbs-dropdown-lessonTextColorSelected: The text color of the selected lesson.@donmckenna If we agree on these new tokens, do we need to document them in figma?