feat(Accordion): support emoji icon#1492
Conversation
| /** @see https://docs-v5.fontawesome.com/web/reference-icons */ | ||
| const FA_PREFIXES = new Set(['fab', 'fad', 'far']); |
There was a problem hiding this comment.
We use the @ui/EmojiPicker component in the Accordion wysiwyg block in the MdxishEditor in the monorepo. @ui/EmojiPicker allows a user to select between solid and duotone font awesome variants and will emit values with these class prefixes to differentiate those variants.
So the icon prop could receive values like fad fa-smile or far fa-smile instead of just fa-smile.
Accounting for that here and falling back to duotone if no prefix is provided.
There was a problem hiding this comment.
do we not need to support fal and fas as well?
maximilianfalco
left a comment
maximilianfalco
left a comment
There was a problem hiding this comment.
lgtm! tested this both in the monorepo and the demo app and they both work nicely! just one non blocking question about the FA prefixes!
| /** @see https://docs-v5.fontawesome.com/web/reference-icons */ | ||
| const FA_PREFIXES = new Set(['fab', 'fad', 'far']); |
There was a problem hiding this comment.
do we not need to support fal and fas as well?
eaglethrost
left a comment
eaglethrost
left a comment
There was a problem hiding this comment.
@kevinports Did some quick fontawesome research and I think you need to also include 'fas', 'fal', 'fat' in the FA_PREFIXES as well as it won't render the icon anymore.
Given these accordions:
<Accordion icon="fas fa-star" title="Solid star (fas)">
</Accordion>
<Accordion icon="fal fa-book" title="Light book (fal)">
</Accordion>
<Accordion icon="fat fa-bell" title="Thin bell (fat)">
</Accordion>Before:
After:
Other than that, the changes work nicely!
| <details className="Accordion" onToggle={() => setIsOpen(!isOpen)}> | ||
| <summary className="Accordion-title"> | ||
| <i className={`Accordion-toggleIcon${isOpen ? '_opened' : ''} fa fa-regular fa-chevron-right`}></i> | ||
| {icon && <i className={`Accordion-icon fa-duotone fa-solid ${icon}`} style={{ color: `${iconColor}` }}></i>} |
There was a problem hiding this comment.
Is it fine to not have the fa-duotone class anymore?
3 participants
🎯 What does this PR do?
Adds emoji icon support to the
Accordioncomponent alongside existing Font Awesome icon support.This is necessary because the new WYSIWYG accordion UI in our editor supports selecting both Font Awesome and emoji icons https://github.com/readmeio/readme/pull/19025
🧪 QA tips
Paste each snippet into a ReadMe doc and confirm the accordion renders correctly.
Emoji icon
🚀emoji appears to the left of the titleFont Awesome icon (existing behavior, no regression)
Font Awesome icon with color (no regression)
iconColorNo icon (no regression)
📸 Screenshot or Loom