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

[Chore] Update button loading prop usage #125

Merged
merged 9 commits into from
Jun 28, 2019

Conversation

michaeljaltamirano
Copy link
Contributor

What & Why

  1. Corrects warnings logged in Update emotion dependencies #124 related to passing non-boolean loading attribute to the DOM by renaming all usage in <Button /> and <RoundButton /> to isLoading.
  2. Adds a deprecation notice to use of loading

Other

  • Once this is merged in, I intend to release Radiance with a minor bump to update our emotion use. I'll then make the necessary changes in PocketDerm before then removing the deprecation warning. (Gatsby does not use this prop, though it looks like it is present in exactly one snapshot, due to it being a defaultProp).

@michaeljaltamirano michaeljaltamirano changed the title Chore/update button loading usage [Chore] Update button loading prop usage Jun 28, 2019
<Button loading>Primary Loading</Button>
<Button loading buttonType="secondary">
<Button isLoading>Primary Loading</Button>
<Button isLoading buttonType="secondary">
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 tested that the basic loading functionality worked by changing these locally to be loading and seeing no regressions.

icon,
textColor,
...rest
} = this.props;

const loadingVal = loading === undefined ? isLoading : loading;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Core of the backwards compatibility.

Copy link
Contributor

@snags88 snags88 left a comment

Choose a reason for hiding this comment

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

🥚 cellent

tenor-181116566

Copy link
Contributor

@daigof daigof left a comment

Choose a reason for hiding this comment

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

code looks good. I notice we merged some other PR without a Radiance release, could you check all the merged commits after the last release, for example Ben merged a new font constant yesterday we need to include that in the release notes. Or let me know I can process a release as well

@michaeljaltamirano michaeljaltamirano merged commit a6a5b35 into master Jun 28, 2019
@michaeljaltamirano michaeljaltamirano deleted the chore/update-button-loading-usage branch June 28, 2019 18:35
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