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

Card subcomponents are missing textAlign props #2027

Closed
levithomason opened this issue Aug 30, 2017 · 4 comments
Closed

Card subcomponents are missing textAlign props #2027

levithomason opened this issue Aug 30, 2017 · 4 comments

Comments

@levithomason
Copy link
Member

levithomason commented Aug 30, 2017

Per SUI core, a Card's children can be aligned:

https://github.com/Semantic-Org/Semantic-UI/blob/master/dist/components/card.css#L203

<Card>
  <Card.Content textAlign="center">
    This should be centered.
  </Card.Content>
</Card>

This is true for all Card subcomponents.

@itamar244
Copy link
Contributor

itamar244 commented Aug 31, 2017

I am trying to do this change.
Should I implement this to every Card.* Component or do it somehow generally?
The propType is PropTypes.oneOf(['left', 'center', 'right']) (the is SUI.TEXT_ALIGNMENTS constant but in the css file you listed above the isn't a justified class)?

Also I'm a new contributor, can you please help me out with commit message?
i have problems with adding tests too. Need a little help.

@itamar244
Copy link
Contributor

itamar244 commented Aug 31, 2017

Solved the tests.
Do I need to add typescript definitions?
Also there isn't a type of 'left' | 'center' | 'right'

@levithomason
Copy link
Member Author

Awesome! Go ahead an open a PR so folks know this is taken. In your PR description, please note that it Fixes #2027 so it auto closes this issue on merge 👍


Should I implement this to every Card.* Component

Yes, please.

the is SUI.TEXT_ALIGNMENTS constant but in the css file you listed above the isn't a justified class

See Segment.js for example:

PropTypes.oneOf(_.without(SUI.TEXT_ALIGNMENTS, 'justified')),

Do I need to add typescript definitions? Also there isn't a type of 'left' | 'center' | 'right'.

Yes, please. We are just listing them explicitly, see Table typings for example.

textAlign?: 'center' | 'left' | 'right';

@itamar244
Copy link
Contributor

Opened a PR and finished changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants