feat: Allow <Pin> glyphs to be passed as children (close #98) #99
Conversation
src/components/pin.tsx
Outdated
| /** | ||
| * Set glyph to glyph container if children are present (rendered via portal). | ||
| * If both props.glyph and props.children are present, props.children takes priority. | ||
| * */ |
There was a problem hiding this comment.
This shouldn't be using jsdoc-style comments.
| /** | |
| * Set glyph to glyph container if children are present (rendered via portal). | |
| * If both props.glyph and props.children are present, props.children takes priority. | |
| * */ | |
| // Set glyph to glyph container if children are present (rendered via portal). | |
| // If both props.glyph and props.children are present, props.children takes priority. |
There was a problem hiding this comment.
fixed in 0aa04d8
| <Pin background={'#22ccff'} borderColor={'#1e89a1'} scale={1.4}> | ||
| {/* child gets rendered as 'glyph' element of pin */} | ||
| <img | ||
| src="https://www.svgrepo.com/show/522904/info-circle.svg" |
There was a problem hiding this comment.
I would prefer it to have the svg-file in the example directory.
There was a problem hiding this comment.
Okay, will change it, just did this as a suggestion as I wasn't sure about how you want to handle it in this repo. Any preference for what svg to choose for the example? I just picked a copyright free one from svgrepo.
There was a problem hiding this comment.
I'm not sure if you have a preference for where you want the svg file locally, I put it in an "assets" folder inside the example now in f4d79a0
|
Tested with a few different configurations but did not see a problem with the extra div, I'm not sure what edge cases this could cause a problem in but I didn't run into them |
|
That's fine with me then. |
This PR allows the
glyphproperty of to be passed via JSX children. Functionality is implemented like discussed in #98.It uses a div container for the portal similar to (where the memo'd div wasn't added as a dependency to the useEffect, I assume that was intentional and also implemented it in this way for ).
I don't think theres any benefit to using a more detailed type for props to differentiate if children were passed or not, as those are inaccurate due to typescript/tsx limitations unfortunately, so I just wrapped the existing type with
PropsWithChildren.I took the code style of the other components as reference, but if there's anything to change im happy to do so :)