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.

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

EuiTitle wipes out className of cloned child #2925

Closed
cchaos opened this issue Feb 25, 2020 · 3 comments · Fixed by #2926
Closed

EuiTitle wipes out className of cloned child #2925

cchaos opened this issue Feb 25, 2020 · 3 comments · Fixed by #2926

Comments

@cchaos
Copy link
Contributor

cchaos commented Feb 25, 2020

EuiTitle is a component that doesn't wrap its children, but clones them applying the appropriate classes. However, it doesn't take into account if the child also has a className applied.

Example

Component code written as:

<EuiTitle size="s">
  <h3 className="myClass">My title</h3>
</EuiTitle>

Results in:

<h3 class="euiTitle euiTitle--small">My title</h3>

When it should be:

<h3 class="euiTitle euiTitle--small myClass">My title</h3>

Offending lines:

const classes = classNames(
'euiTitle',
titleSizeToClassNameMap[size],
textTransform ? textTransformToClassNameMap[textTransform] : undefined,
className
);
const props = {
className: classes,
...rest,
};
return React.cloneElement(children, props);

@anishagg17
Copy link
Contributor

@cchaos please assign me this issue. I am working on It

@myasonik
Copy link
Contributor

@cchaos What do you think about EuiTitle taking a headingLevel param to not require html be passed in at all?

@cchaos
Copy link
Contributor Author

cchaos commented Feb 25, 2020

I think that's out of scope for this issue and would require a larger refactor plus major breaking changes that I don't think are necessary given the wide usage of this component.

thompsongl added a commit that referenced this issue Feb 26, 2020
* added child class to wraper

* added test suite

* Update src/components/title/title.test.tsx

Co-Authored-By: Greg Thompson <[email protected]>

* Update CHANGELOG.md

Co-Authored-By: Caroline Horn <[email protected]>

* Updated SnapShots

Co-authored-by: Greg Thompson <[email protected]>
Co-authored-by: Caroline Horn <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants