Add component prop to EuiTextColor.#1011
Conversation
- Fix bug: `EuiDescribedFormGroup` renders its `description` inside of a `div` instead of a `span`.
| if (color) { | ||
| optionallyAlteredText = ( | ||
| <EuiTextColor color={color}> | ||
| <EuiTextColor color={color} component="div"> |
There was a problem hiding this comment.
I don't think I'd expect EuiText to render as a div, this seems like it would have much wider implications.
There was a problem hiding this comment.
Do you mean EuiTextColor, not EuiText? Since the parent element is a div, rendering another div inside of it seems OK to me... what implications do you foresee?
There was a problem hiding this comment.
Ah, I didn't notice that the parent element here is a div. Really didn't expect that :)
There was a problem hiding this comment.
I think this makes sense and I'm trying to think of any scenarios where previous implementations might break because of this change, but can't really do so.
chandlerprall
left a comment
chandlerprall
left a comment
There was a problem hiding this comment.
LGTM, pulled and tested docs
|
I'm confused. Mainly I'm just trying to understand what the bug was. I see in |
|
Ah sorry! Here's a PR with a screenshot that illustrates the buggy scenario: elastic/kibana#20744. In Kibana's Advanced Settings, we pass block-level elements to the |
|
Ahh it's when you're trying to pass multiple different types of elements to the description. Ok. |
| if (color) { | ||
| optionallyAlteredText = ( | ||
| <EuiTextColor color={color}> | ||
| <EuiTextColor color={color} component="div"> |
There was a problem hiding this comment.
I think this makes sense and I'm trying to think of any scenarios where previous implementations might break because of this change, but can't really do so.
| children: PropTypes.node, | ||
| className: PropTypes.string, | ||
| color: PropTypes.oneOf(COLORS), | ||
| component: PropTypes.oneOf(['div', 'span']), |
There was a problem hiding this comment.
A comment here would be nice.
3 participants
This fixes a bug, so that now
EuiDescribedFormGrouprenders itsdescriptioninside of adivinstead of aspan. The original behavior caused issues in advanced settings because divs would end up rendered inside the span.