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

EuiTextAlign and EuiTextFontFamily components #249

Closed
wants to merge 3 commits into from

Conversation

snide
Copy link
Contributor

@snide snide commented Dec 22, 2017

Two new components for text manipulation. Similar to EuiTextColor these props can also be passed directly onto EuiText itself for an all in one solution.

image

Copy link
Contributor

@cjcenizal cjcenizal left a comment

Choose a reason for hiding this comment

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

Looks good. How about we combine these two components into a single EuiTextFontFormat component, which has fontFamily and align props? This seems like it will be a bit easier to consume and scale.

@uboness
Copy link
Contributor

uboness commented Dec 23, 2017

Looks good. How about we combine these two components into a single EuiTextFontFormat component, which has fontFamily and align props? This seems like it will be a bit easier to consume and scale.

or... EuiTextFormat that will take care of anything text formatting that we'd like to expose... so we can scale with it in the future as well (if more formatting needs will come up)

@snide
Copy link
Contributor Author

snide commented Dec 23, 2017

@cjcenizal @uboness EuiText already is that all in one consumer (which accepts props for color, textAlign and fontFamily, and I think would be applicable for the majority of usages (aka: places with a single line of text) you would need.

Example:

<EuiText fontFamily="code" color="danger" textAlign="right">
  <p>Hey, I'm some red monospaced text on the right side</p>
</EuiText>

We still need a separate component (either separate or an all in one component) for situations where you need to nest styling.

<EuiText>
  <EuiTextAlign textAlign="center">
    <h3>I'm a centered title</h3>
  </EuiTextAlign>
  <EuiTextColor color="danger">
    <p>Hello, I'm red!</p>
  </EuiTextColor>
</EuiText>

I'm not opposed to wrapping color, family and alignment into an EuiTextStyle component if that's what we want so it would be something like this instead...

<EuiText>
  <EuiTextStyle textAlign="center">
    <h3>I'm a centered title</h3>
  </EuiTextStyle>
  <EuiTextStyle color="danger">
    <p>Hello, I'm red!</p>
  </EuiTextStyle>
</EuiText>

Lemme know if that works more with what you're thinking and I'll adjust the PR.

@snide
Copy link
Contributor Author

snide commented Jan 8, 2018

@cjcenizal and I chatted about this one quickly and decided to go with the last example making all text style related concerns under a single component separate from EuiText which handles formatting. I'll refactor and get this PR back up to speed.

<EuiText>
  <EuiTextStyle textAlign="center">
    <h3>I'm a centered title</h3>
  </EuiTextStyle>
  <EuiTextStyle color="danger">
    <p>Hello, I'm red!</p>
  </EuiTextStyle>
</EuiText>

@snide
Copy link
Contributor Author

snide commented Apr 18, 2018

Closing in favor of #683

@snide snide closed this Apr 18, 2018
@snide snide deleted the feature/text_align_and_family branch April 18, 2018 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants