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

Accept number for size prop in ActivityIndicator #8846

Closed

Conversation

fadils
Copy link
Contributor

@fadils fadils commented Jul 17, 2016

motivation

Previously, size can only accept either 'small' or 'large'. And to obtain a custom size, scale transformation is used. This is to let users to possibly pass number value directly to define ActivityIndicator's size.

Test plan

I have also modified the current example to reflect the new size prop in action.

@ghost
Copy link

ghost commented Jul 17, 2016

By analyzing the blame information on this pull request, we identified @janicduplessis and @javache to be potential reviewers.

@ghost
Copy link

ghost commented Jul 17, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost ghost added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Jul 17, 2016
size="large"
color="#aa3300"
size='large'
color='#aa3300'
/>

Choose a reason for hiding this comment

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

jsx-quotes: Unexpected usage of singlequote.

@ghost
Copy link

ghost commented Jul 17, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 7b5970f to 0b4d00f Compare July 17, 2016 10:20
}
},
{
title: 'Custom size (size: 30, scale transform: 1.5)',
render() {
return (
<ActivityIndicator
style={[styles.centering, {transform: [{scale: 1.5}]}]}

Choose a reason for hiding this comment

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

number This type is incompatible with string

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
@fadils fadils changed the title Activity indicator/add size prop Accept number for size prop in ActivityIndicator Jul 17, 2016
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
styleAttr="Normal"
indeterminate
/>
{ReturnedActivityIndicator}
Copy link
Contributor

Choose a reason for hiding this comment

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

you don't need to conditionally set the RCTActivityIndicator. just setting the right sizeStyle variable should be enough

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks @chirag04 !

@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 0b4d00f to 63af220 Compare July 17, 2016 13:55
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
@@ -56,7 +56,7 @@ const ToggleAnimatingActivityIndicator = React.createClass({
<ActivityIndicator
animating={this.state.animating}
style={[styles.centering, {height: 80}]}
size="large"
size='large'

Choose a reason for hiding this comment

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

jsx-quotes: Unexpected usage of singlequote.

@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 63af220 to 587b07a Compare July 17, 2016 14:08
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 587b07a to 1d7c3be Compare July 17, 2016 14:12
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 1d7c3be to 7df46a9 Compare July 17, 2016 14:40
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
return (
<ActivityIndicator
style={styles.centering}
size={48}

Choose a reason for hiding this comment

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

number This type is incompatible with string

@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 7df46a9 to a4a0d8d Compare July 17, 2016 17:39
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 17, 2016
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from a4a0d8d to de373a3 Compare July 18, 2016 15:03
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 18, 2016
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from c94edb1 to 825343c Compare July 21, 2016 00:54
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 21, 2016
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from 825343c to a0e189a Compare July 21, 2016 01:35
@ghost ghost added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 21, 2016
fadils added 4 commits July 21, 2016 08:40
Previously, size can only accept either 'small' or 'large'.
And to obtain a custom size, scale transformation is used.
This is to let users to possibly pass number value to define
ActivityIndicator's size.
@fadils fadils force-pushed the ActivityIndicator/add-size-prop branch from a0e189a to 6e91039 Compare July 21, 2016 01:43
@fadils fadils closed this Jul 21, 2016
var size = props[propName];

if (size !== undefined && size !== null && typeof size !== 'number' && size !== 'small' && size !== 'large') {
var locationName = ReactPropTypeLocationNames[location];

Choose a reason for hiding this comment

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

no-unused-vars: 'locationName' is defined but never used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants