Skip to content

fix: align card actions to the left - #262

Merged
satya164 merged 5 commits into
callstack:masterfrom
gawrysiak:fix/card-actions-align
Mar 3, 2018
Merged

satya164 merged 5 commits into
callstack:masterfrom
gawrysiak:fix/card-actions-align

Conversation

@gawrysiak

@gawrysiak gawrysiak commented Mar 3, 2018

Copy link
Copy Markdown
Collaborator

This is a suggestion to change the CardActions align from right to left. Additionally, it adds a "right" prop to easily go back to the original behavior, without changing any CSS.

Motivation

It seems like the original Material Design guidelines aligns the actions to the left by default. This is as well the standard behavior for other libs, like material-ui.

@callstack-bot

Copy link
Copy Markdown

Hey @gawrysiak, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

Comment thread src/components/Card/CardActions.js Outdated
},
});
render() {
const styles = StyleSheet.create({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't create stylesheets in render, move it to outside. Use inline styles for styles based on props.

Comment thread src/components/Card/CardActions.js Outdated
/**
* Should actions be aligned to the right.
*/
right?: boolean,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we want to provide a prop which doesn't align with material design guidelines. Let's align them to left by default and remove this prop.

@satya164
satya164 merged commit 3a2f3f8 into callstack:master Mar 3, 2018
@gawrysiak
gawrysiak deleted the fix/card-actions-align branch March 3, 2018 11:24
eriveltonelias pushed a commit to eriveltonelias/react-native-paper that referenced this pull request Sep 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants