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

[BPK-1043] Remove selected button state #188

Merged
merged 1 commit into from
Oct 11, 2017

Conversation

shaundon
Copy link
Contributor

And also examples of buttons with both text and icon (not the code, just the references to it in docs, as discussed).

And also examples of buttons with both text and icon.
@backpackbot
Copy link

backpackbot commented Oct 10, 2017

Warnings
⚠️

This PR contains 13 files (0 new, 13 modified). Consider splitting it into multiple PRs.

Messages
📖

Thanks for the PR 🎉.

Generated by 🚫 dangerJS

@shaundon shaundon force-pushed the bpk-1043-remove-selected-state branch from 5ee0de9 to 33c8838 Compare October 10, 2017 16:33
@@ -44,11 +44,9 @@ export default class App extends Component {
<View style={styles.container}>
<BpkButton type="primary" title={translationHelper.translate('BOOK_FLIGHT')} onPress={() => {}} />
<BpkButton type="featured" title={translationHelper.translate('BOOK_FLIGHT')} onPress={() => {}} />
<BpkButton selected type="secondary" title={translationHelper.translate('BOOK_FLIGHT')} onPress={() => {}} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing the selected one.

<BpkButton disabled type="destructive" title={translationHelper.translate('BOOK_FLIGHT')} onPress={() => {}} />
<BpkButton large type="primary" title={translationHelper.translate('BOOK_FLIGHT')} onPress={() => {}} />

<BpkButton type="primary" title={translationHelper.translate('BOOK_FLIGHT')} icon={ArrowImage} onPress={() => {}} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing example of a button with both text and icon.

</View>
</BVLinearGradient>
`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't removed tests for buttons that have both text and an icon, because as discussed we're not removing it, just not advertising the functionality.

Copy link
Contributor

@k0nserv k0nserv left a comment

Choose a reason for hiding this comment

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

LGTM

@shaundon shaundon merged commit cc9513b into master Oct 11, 2017
@shaundon shaundon deleted the bpk-1043-remove-selected-state branch October 11, 2017 10:37
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.

4 participants