feat(StringFormatter): new component StringFormatter #4249
Conversation
updated css Updated DefinitionTooltip import (incorrect name) Small changes to V11 component Minor changes to proptype comments Added overrides for tooltip overlay size and position
✅ Deploy Preview for carbon-for-ibm-products ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Modified things for the stories.
Added description to the component Updated the docs mdx page
jlongshore
added
version: 2
jlongshore
added
status: waiting for maintainer 💬
and removed
version: 2
There was a problem hiding this comment.
looks good! have a few comments on the code and one regarding the style
the line-height here seems a bit small? i don't see any design documentation that's accompanying this, so i don't know for certain. this might be nothing, but i just wanted to point out that it looks a bit squished.
| export let StringFormatter = React.forwardRef( | ||
| ( | ||
| { | ||
| // The component props, in alphabetical order (for consistency). |
There was a problem hiding this comment.
same comment as before about cleaning up comments
| ref={ref} | ||
| { | ||
| // Pass through any other property values as HTML attributes. | ||
| ...rest |
There was a problem hiding this comment.
prop spreading should always be first in order to avoid the user possibly overwriting them
| * Only used internally | ||
| */ | ||
|
|
||
| let StringFormatterContent = React.forwardRef( |
There was a problem hiding this comment.
as a general convention we should always separate components into their own files. this seems small enough that i don't think it's strictly necessary to have it's own dedicated file. i think this code could just be lumped back into StringFormatter. either way is completely fine.
Put subcomponent into its own file
Yep, this thing should just inherit the line height, color - that kinda stuff from its parent or be set with a class or something. I think for the stories we just did body-01 type style and a text-primary color |
74d67d0
Contributes to issue 4224
Migrate StringFormatter from
Products v1 / Security / StringFormatterto Products v2 asStringFormatter.v1References: GitHub, Storybook.What did you change?
How did you test and verify your work?