Skip to content

Conversation

@Monte9
Copy link
Collaborator

@Monte9 Monte9 commented Jul 14, 2017

Major Breaking changes

Please see the /example app for the above implementation.

Really simple and useful API:

Button.propTypes = {
  text: PropTypes.string,
  textStyle: PropTypes.object,
  textProps: PropTypes.object,

  buttonStyle: ViewPropTypes.style,

  loading: PropTypes.bool,
  loadingStyle: ViewPropTypes.style,
  loadingProps: PropTypes.object,

  onPress: PropTypes.any,
  containerStyle: ViewPropTypes.style,

  icon: PropTypes.object,
  iconContainerStyle: ViewPropTypes.style,
  iconRight: PropTypes.bool,
};

@Monte9
Copy link
Collaborator Author

Monte9 commented Jul 14, 2017

@binoy14 @iRoachie thoughts on this? 🤔

Basically, here's what I had in mind while refactoring:

  1. Simplify the named Props handles by Button.js (42 => 10) 😮
  2. Make the code more legible and modularized ( Icon, for instance, needs to be passed in as an object, because the Button doesn't care about how/where you get your icon from)
  3. Simple and easy to understand button layout with full flexibility by using containerStyle & style props
  4. Implement common use cases as seen in various designs

Here's an example of what a button looks like:

<Button
  text="Add to Cart"
  underlayColor="rgba(80,117,196,1)"
  activeOpacity={0.4}
  textStyle={{fontWeight: 'bold', fontSize: 18}}
  buttonStyle={{backgroundColor: 'rgba(90, 154, 230, 1)', height: 40, width: 200, borderWidth: 0, borderColor: 'transparent', borderRadius: 20}}
  containerStyle={{marginVertical: 10}}
  icon={
    <Icon
      name='arrow-right'
      size={15}
      color='white'
    />
  }
  iconRight
  iconContainerStyle={{marginLeft: 5}}
/>

This will def break existing functionality, but I would rather have a cleaner and simpler API to work with for 1.0 than just patch and band-aid fixes with the current API.

Also here are some useful designs to implement in case anyone wants to play with the Button API.

p.s - if this get's approved, many more breaking changes and refactors to come. ;)

@@ -1,281 +1,182 @@
import PropTypes from 'prop-types';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is the file that needs to be review for the API changes

@nonotest
Copy link
Contributor

+1 for the icon change, hate passing an object for this.
also +1 for the style changes, current api can be confusing

@Monte9
Copy link
Collaborator Author

Monte9 commented Jul 15, 2017

Thanks for the feedback @nonotest!

@xavier-villelegier
Copy link
Collaborator

This includes breaking changes but i think it is worth it. I really like the way you use textProps and loadingProps instead of exposing the entire API of those components through the Button component, I think this could be a good start point to simplify others ! 🔥
+1 for the icon change too, much more modular.

@Monte9
Copy link
Collaborator Author

Monte9 commented Jul 17, 2017

@nonotest @xavier-villelegier thanks for pitching in. I used this API myself to design buttons on the example app and I really like it. I think I have landed on a pretty stable API and it looks ready to be merged to me.

Also I have added a few todo items here.

@Monte9 Monte9 merged commit 6b2a0f0 into v1 Jul 17, 2017
@iRoachie iRoachie deleted the button_v1 branch January 12, 2018 00:40
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.

4 participants