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.

Sign up for GitHub

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

Jump to bottom

[No QA] Remove broken defaultProps #9890

Merged
merged 1 commit into from
Jul 14, 2022
Merged

[No QA] Remove broken defaultProps #9890

merged 1 commit into from
Jul 14, 2022

Conversation

luacmartins
Copy link
Contributor

@luacmartins luacmartins commented Jul 13, 2022 •
edited
Loading

Details

Removes broken default props from IOUAmountPage. If any key in props.iou is returned, the defaultProps for iou doesn't actually check that iou.selectedCurrencyCode is set, which makes it useless.

cc @mountiny @PauloGasparSv

Fixed Issues

$ #9889

Tests

  1. Comment this line
  2. Log into an account
  3. Navigate to + > Request money
  4. Verify that you see the default USD currency
  • Verify that no errors appear in the JS console

@luacmartins luacmartins self-assigned this Jul 13, 2022
@luacmartins luacmartins marked this pull request as ready for review July 13, 2022 21:20
@luacmartins luacmartins requested a review from a team as a code owner July 13, 2022 21:20
@luacmartins luacmartins removed the request for review from a team July 13, 2022 21:20
@melvin-bot melvin-bot bot requested a review from amyevans July 13, 2022 21:21
amyevans
amyevans approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@amyevans amyevans left a comment

Choose a reason for hiding this comment

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

Alternatively you could set a defaultProp for selectedCurrencyCode on , but it's kinda 🥔 🥔

@luacmartins luacmartins merged commit c3bb527 into main Jul 14, 2022
@luacmartins luacmartins deleted the cmartins-cleanDefaultProps branch July 14, 2022 15:12
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

@OSBotify OSBotify mentioned this pull request Jul 18, 2022
Closed
79 tasks
@OSBotify
Copy link
Contributor

🚀 Deployed to production by @luacmartins in version: 1.1.85-8 🚀

platform result
🤖 android 🤖 success ✅
🖥 desktop 🖥 success ✅
🍎 iOS 🍎 success ✅
🕸 web 🕸 success ✅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PauloGasparSv PauloGasparSv PauloGasparSv approved these changes

@amyevans amyevans amyevans approved these changes

@mountiny mountiny Awaiting requested review from mountiny

Assignees

@luacmartins luacmartins

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@luacmartins @OSBotify @PauloGasparSv @amyevans