Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add component prop to EuiTextColor. #1011

Merged
merged 2 commits into from
Jul 13, 2018
+24 −10
Merged

Add component prop to EuiTextColor. #1011

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6b83f3
Add `component` prop to `EuiTextColor`.
cjcenizal Jul 13, 2018
66029de
Add comment.
cjcenizal Jul 13, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,19 @@
- Make some proprties of `EuiFlyout` optional ([#1003](https://github.com/elastic/eui/pull/1003))
- Add typings for `EuiFlyout`, `EuiFlyoutBody`, `EuiFlyoutHeader`, and `EuiFlyoutFooter` ([#1001](https://github.com/elastic/eui/pull/1001))
- Gave `EuiFlyout` close button a data-test-subj ([#1000](https://github.com/elastic/eui/pull/1000))
- Updated `react-vis` version to `1.10.2`. ([#999](https://github.com/elastic/eui/pull/999))
- Updated `react-vis` version to `1.10.2` ([#999](https://github.com/elastic/eui/pull/999))
- Added `component` prop to `EuiTextColor` ([#1011](https://github.com/elastic/eui/pull/1011))

**Breaking changes**

- Altered `EuiPage` and sub-component layout ([#998](https://github.com/elastic/eui/pull/998))
- `EuiPageHeader` must now be contained within `EuiPageBody`
- `EuiPageSideBar` must now be **outside** of `EuiPageBody`

**Bug fixes**

- `EuiDescribedFormGroup` now renders its `description` inside of a `div` instead of a `span` ([#1011](https://github.com/elastic/eui/pull/1011))

## [`1.2.1`](https://github.com/elastic/eui/tree/v1.2.1)

**Bug fixes**
Expand Down
9 changes: 5 additions & 4 deletions src-docs/src/views/form_layouts/described_form_group.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import React, {
Component,
Fragment,
} from 'react';

import {
Expand Down Expand Up @@ -81,11 +82,11 @@ export default class extends Component {
idAria="single-example-aria"
title={<h3>Single text field</h3>}
description={
<span>
<Fragment>
When using this with a single form row where this text serves as the help text for the input,
it is a good idea to pass <EuiCode>idAria=&quot;someID&quot;</EuiCode> to the form group and pass
<EuiCode>describedByIds=&#123;[someID]&#125;</EuiCode> to its form row.
</span>
</Fragment>
}
>
<EuiFormRow
Expand Down Expand Up @@ -141,11 +142,11 @@ export default class extends Component {
title={<h2>Full width</h2>}
titleSize="xxxs"
description={
<span>
<Fragment>
By default, <EuiCode>EuiDescribedFormGroup</EuiCode> will be double the default width of form elements.
However, you can pass <EuiCode>fullWidth</EuiCode> prop to this, the individual field and row components
to expand to their container.
</span>
</Fragment>
}
fullWidth
>
Expand Down
5 changes: 3 additions & 2 deletions src/components/form/described_form_group/__snapshots__/described_form_group.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,13 @@ exports[`EuiDescribedFormGroup ties together parts for accessibility 1`] = `
>
<EuiTextColor
color="subdued"
component="div"
>
<span
<div
className="euiTextColor euiTextColor--subdued"
>
Test description
</span>
</div>
</EuiTextColor>
</div>
</EuiText>
Expand Down
2 changes: 1 addition & 1 deletion src/components/text/text.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const EuiText = ({ size, color, grow, textAlign, children, className, ...
let optionallyAlteredText;
if (color) {
optionallyAlteredText = (
<EuiTextColor color={color}>
<EuiTextColor color={color} component="div">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think I'd expect EuiText to render as a div, this seems like it would have much wider implications.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I didn't notice that the parent element here is a div. Really didn't expect that :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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}
</EuiTextColor>
);
Expand Down
7 changes: 5 additions & 2 deletions src/components/text/text_color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ export const EuiTextColor = ({
children,
color,
className,
component: Component,
...rest
}) => {
const classes = classNames(
Expand All @@ -27,21 +28,23 @@ export const EuiTextColor = ({
);

return (
<span
<Component
className={classes}
{...rest}
>
{children}
</span>
</Component>
);
};

EuiTextColor.propTypes = {
children: PropTypes.node,
className: PropTypes.string,
color: PropTypes.oneOf(COLORS),
component: PropTypes.oneOf(['div', 'span']),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment here would be nice.

};

EuiTextColor.defaultProps = {
color: 'default',
component: 'span',
};