feature: [Text] Add isParagraph props#60
Conversation
nicosampler
requested review from
Agupane,
alongoni,
dasanra,
fernandomg and
mmv08
ESLint Summary View Full Report
Report generated by eslint-plus-action |
|
Travis automatic deployment: |
mmv08
left a comment
mmv08
left a comment
There was a problem hiding this comment.
I think this would be better if we would add a prop for element. E.g <Text is='span' />. Boolean API like this would make adding other elements harder.
Also there are other libraries with such API and imo it would be more familiar for the developers
|
@mikheevm Agreed with the syntax, but I can not imagine another component to be used furthermore than p and span. If you can imagine another, I will change the prop name. |
|
@nicosampler |
|
we are already covering some of them but agreed that some others could be supported as well in the future. |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
src/dataDisplay/Text/index.tsx
Outdated
| HTMLParagraphElement | HTMLSpanElement, | ||
| Props | ||
| >(({ is = 'paragraph', children, ...rest }, ref) => | ||
| is === 'paragraph' ? ( |
There was a problem hiding this comment.
There was a problem hiding this comment.
This produces a lot of problems with typescript, how would you pass the Props and also with the ref.
how do you suggest to fix them?
There was a problem hiding this comment.
https://styled-components.com/docs/api#as-polymorphic-prop
looks like styled-components has support for that. Could you please check it again and see if any errors pop up?
There was a problem hiding this comment.
works like a charm!
|
Travis automatic deployment: |
|
Travis automatic deployment: |
|
Travis automatic deployment: |
3 participants
This Pull Request adds a prop
isParagraphtoTextcomponent.When
isParagraph(default true) Text is wrapped into a<p>element.When
isParagraphis false, Text is Wrapped into a<span>element.