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

[iOS][Fix] Do not override ActivityIndicator color when setting its size #25849

Closed
wants to merge 1 commit into from

Conversation

cabelitos
Copy link
Contributor

@cabelitos cabelitos commented Jul 27, 2019

Summary

The activityIndicatorViewStyle property overrides the previous set color
if it's changed. Depending on the property set order you may end in a state
that the color property will never be respected since it first sets
the color and then the activityIndicatorViewStyle property (which overrides
the color property). In order to prevent this problem
before setting the new activityIndicatorViewStyle save the old
color and override it after activityIndicatorViewStyle is set. Thus
always respecting the user's color.

Changelog

[iOS] [Fixed] - Do not override ActivityIndicator color when setting its size

Test Plan

Using the code below on iOS notice that the last ActivityIndicator will always have its color set to white while te testID is provided

Without the patch

Notice the white -> blue transition when disabling the testID

broken

With the patch

Color remains unchanged

working

import React from "react";
import { View, StyleSheet, ActivityIndicator, Button } from "react-native";

const App = () => {
  const [enableTestID, onSetEnableTestID] = React.useState(true);
  const onPress = React.useCallback(() => {
    onSetEnableTestID(!enableTestID);
  }, [enableTestID]);
  return (
    <View style={styles.container}>
      <ActivityIndicator size="large" color="red" />
      <ActivityIndicator size="small" color="red" />
      <ActivityIndicator size="small" />
      <ActivityIndicator color="green" />
      <ActivityIndicator
        key={enableTestID.toString()}
        size="large"
        color="blue"
        testID={enableTestID ? 'please work' : undefined}
      />
      <Button
        title={enableTestID ? 'Disable testID' : 'enable testID'}
        onPress={onPress}
      />
    </View>
  );
};

export default App;

const styles = StyleSheet.create({
  container: {
    flex: 1,
    alignItems: "center",
    justifyContent: "center",
    backgroundColor: "black"
  },
});

Closes #25319

…s size

The activityIndicatorViewStyle property overrides the previous set color
if it's changed. Depending on the property set order you may end in a state
that the color property will never be respected since it first sets
the color and then the activityIndicatorViewStyle property (which overrides
the color property). In order to prevent this problem
before setting the new activityIndicatorViewStyle save the old
color and override it after activityIndicatorViewStyle is set. Thus
always respecting the user's color.
@cabelitos cabelitos requested a review from shergin as a code owner July 27, 2019 20:39
@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 27, 2019
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sammy-SC has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@sammy-SC
Copy link
Contributor

@cabelitos thank you for fixing this!

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @cabelitos in 14b0ed4.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Jul 30, 2019
@cabelitos
Copy link
Contributor Author

@sammy-SC thank you very much for your approval! glad to help;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Component: ActivityIndicator Merged This PR has been merged. Platform: iOS iOS applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ActivityIndicator breaks when adding a testID
4 participants