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

feat(Bonus Pagamenti Digitali): [#176240699,#175891337,#176166370] Convert BPD activation status to pot, prevent cashback logo flickering on Android #2639

Merged
merged 8 commits into from
Dec 29, 2020

Conversation

fabriziofff
Copy link
Contributor

@fabriziofff fabriziofff commented Dec 23, 2020

Short description

This pr converts bpd activation status from RemoteValue to pot and memoize the BpdCardsInWalletContainer component, in order to avoid useless rendering and prevent the cashback logo flickering on Android.

List of changes proposed in this pull request

  • Change bonus/bpd/details/activation/enabled from RemoteValue to pot
  • Change BpdCardsInWalletContainer in order to avoid re-rendering with the same values
  • Changed the auto-refresh time for BpdCardsInWalletContainer, from 30 seconds to `5 minutes

@pagopa-github-bot pagopa-github-bot changed the title [#176240699] Convert BPD activation status to pot feat(Bonus Pagamenti Digitali): [#176240699] Convert BPD activation status to pot Dec 23, 2020
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Dec 23, 2020

Warnings
⚠️

Multiple stories with different types are associated with this Pull request.
Only one tag will be added, following the order: feature > bug > chore

Affected stories

  • 🌟 #176240699: Come DEV voglio che lo stato di attivazione di BPD sia un pot invece che un RemoteValue
  • 🐞 #175891337: [minor] CAsPA184374 Su Android il logo "cashback" nella card preview nel wallet, sfarfalla
  • 🌟 #176166370: Come DEV voglio evitare di richiedere la lista dei periodi e gli amount ad ogni refresh

Generated by 🚫 dangerJS against f4c04ec

@codecov
Copy link

codecov bot commented Dec 23, 2020

Codecov Report

Merging #2639 (f4c04ec) into master (e0f80f6) will decrease coverage by 0.01%.
The diff coverage is 68.75%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2639      +/-   ##
==========================================
- Coverage   51.09%   51.08%   -0.02%     
==========================================
  Files         721      721              
  Lines       20537    20540       +3     
  Branches     3950     3632     -318     
==========================================
- Hits        10493    10492       -1     
- Misses      10000    10004       +4     
  Partials       44       44              
Impacted Files Coverage Δ
ts/features/bonus/bpd/model/RemoteValue.ts 56.52% <ø> (ø)
ts/screens/wallet/WalletHomeScreen.tsx 25.11% <0.00%> (-0.35%) ⬇️
.../walletCardContainer/BpdCardsInWalletComponent.tsx 58.62% <33.33%> (-3.29%) ⬇️
...nus/bpd/store/reducers/details/activation/index.ts 51.02% <37.50%> (+1.02%) ⬆️
...onus/bpd/components/BpdPaymentMethodCapability.tsx 58.13% <66.66%> (ø)
...tures/bonus/bpd/store/reducers/details/combiner.ts 56.06% <66.66%> (-0.66%) ⬇️
...features/wallet/component/FeaturedCardCarousel.tsx 44.06% <83.33%> (-0.94%) ⬇️
ts/sagas/wallet.ts 21.35% <90.90%> (-0.38%) ⬇️
ts/screens/profile/RemoveAccountDetailsScreen.tsx 40.29% <91.66%> (-0.88%) ⬇️
...d/saga/orchestration/onboarding/startOnboarding.ts 83.78% <100.00%> (ø)
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0f80f6...f4c04ec. Read the comment docs.

@fabriziofff fabriziofff marked this pull request as ready for review December 23, 2020 14:56
@fabriziofff fabriziofff changed the title feat(Bonus Pagamenti Digitali): [#176240699] Convert BPD activation status to pot feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot Dec 23, 2020
@fabriziofff fabriziofff changed the title feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot & prevent cashback logo flickering on Android Dec 23, 2020
@fabriziofff fabriziofff changed the title feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot & prevent cashback logo flickering on Android feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot, prevent cashback logo flickering on Android Dec 23, 2020
@fabriziofff fabriziofff changed the title feat(Bonus Pagamenti Digitali): [#176240699,#175891337] Convert BPD activation status to pot, prevent cashback logo flickering on Android feat(Bonus Pagamenti Digitali): [#176240699,#175891337,#176166370] Convert BPD activation status to pot, prevent cashback logo flickering on Android Dec 23, 2020
@@ -330,7 +325,7 @@ class WalletHomeScreen extends React.PureComponent<Props, State> {
if (
this.props.allActiveBonus.length === 0 ||
this.props.allActiveBonus.every(ab => isStrictSome(ab)) ||
isReady(this.props.bpdActiveBonus)
pot.isLoading(this.props.bpdActiveBonus)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why in this case you didn't use pot.isSome?

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 think it is a typo done during porting 😄 wrong for sure! Changed in f297755

@fabriziofff fabriziofff requested a review from debiff December 28, 2020 18:15
@Undermaken Undermaken merged commit 214f241 into master Dec 29, 2020
@fabriziofff fabriziofff deleted the 176240699-convert-activation-status-to-pot branch January 21, 2022 11:45
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.

4 participants