[WIP] - Recharts issue 3113 filterProps #4127
Conversation
| @@ -0,0 +1,99 @@ | |||
| export type SVGElementAttributeTypes = { | |||
There was a problem hiding this comment.
This list should directly reflect https://developer.mozilla.org/en-US/docs/Web/SVG/Element. If there are things that are no longer on that list we can probably clean the list up a bit
There was a problem hiding this comment.
Yes i would clean it, i was planning to do an update after i received some feedback!
| ) => { | ||
| /** | ||
| * If the svg element type is explicitly included, check against the filtered element key map | ||
| * to determine if there are attributes that should only exist on that element type. | ||
| * @todo Add an internal cjs version of https://github.com/wooorm/svg-element-attributes for full coverage. | ||
| */ | ||
| const matchingElementTypeKeys = FilteredElementKeyMap?.[svgElementType] ?? []; | ||
| const svgElementAttributesInCamelCase = getSVGElementAttributesInCamelCase(svgElementAttributes); |
There was a problem hiding this comment.
the props we accept are also in camel case for the same reason (react), why do we need to do any transformation here?
There was a problem hiding this comment.
I created the getSVGElementAttributesInCamelCase cause the svgElementAttributes object structured as follows. I should have simply create the util to return the converted object without having to do the conversion multiple times.
circle: [
'alignment-baseline',
'baseline-shift',
'clip',
'clip-path',
'clip-rule',
'color',
...
]
```There was a problem hiding this comment.
You can probably do the conversion and once and keep it stored as camel case, we don't need to do it at runtime
| const svgElementAttributesInCamelCase = getSVGElementAttributesInCamelCase(svgElementAttributes); | ||
| const matchingElementTypeKeys = svgElementAttributesInCamelCase?.[svgElementType] ?? []; | ||
|
|
||
| const { polygon, polyline, svg, ...rest } = svgElementAttributesInCamelCase; |
There was a problem hiding this comment.
why do these three need to be extracted out?
There was a problem hiding this comment.
this was me, messing with things!
|
Going to close this for now since follow ups haven't been addressed. Can re-open later |
Description
Related Issue
Motivation and Context
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Checklist: