-
Notifications
You must be signed in to change notification settings - Fork 4.2k
feat(js): override icon set for inbox component #8293
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
Conversation
✅ Deploy Preview for dashboard-v2-novu-staging canceled.
|
@novu/js
@novu/nextjs
novu
@novu/react
@novu/react-native
commit: |
| type IconRendererWrapperProps = { | ||
| iconKey: IconKey; | ||
| fallback: JSX.Element; | ||
| class?: string; | ||
| }; | ||
|
|
||
| export const IconRendererWrapper = (props: IconRendererWrapperProps) => { | ||
| let el: HTMLDivElement | undefined; | ||
| let cleanup: (() => void) | undefined; | ||
| const appearance = useAppearance(); | ||
| const customRenderer = () => appearance.icons()?.[props.iconKey]; | ||
|
|
||
| onMount(() => { | ||
| if (el && customRenderer()) { | ||
| cleanup = (customRenderer() as IconRenderer)(el, { class: props.class }); | ||
| } | ||
| }); | ||
|
|
||
| onCleanup(() => { | ||
| cleanup?.(); | ||
| }); | ||
|
|
||
| return ( | ||
| <Show when={customRenderer()} fallback={props.fallback}> | ||
| {/* Render the placeholder div. The user's renderer will populate it. */} | ||
| <span ref={el} /> | ||
| </Show> | ||
| ); |
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.
@LetItRock this is the rendering wrapper utility function for icons
| { | ||
| iconKey, | ||
| }: { | ||
| iconKey?: IconKey; | ||
| } = {} |
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.
Added options object, with an optional iconKey param
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.
Was duplicated with clock
| }; | ||
| } | ||
|
|
||
| export function adaptAppearanceForJs(appearance?: ReactAppearance): JsAppearance | undefined { |
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 function remaps the jsx syntx to novu/js
|
|
||
| if (hasAppearanceKey) { | ||
| return; // Skip reporting if appearanceKey is present, as it is most likely forwarded. | ||
| if (!node.name || node.name.name !== 'class') { |
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.
Fixed the linter to allow using variables with style
LetItRock
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.
💎
What changed? Why was the change needed?
Introduce a new icons key in the
appereanceprop:Icon keys are discovarable via the 🖼️ emoji on the class.
Screenshots
Expand for optional sections
Related enterprise PR
Special notes for your reviewer