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): [#176578587] Cobadge detail screen #2751

Merged
merged 52 commits into from
Feb 2, 2021

Conversation

debiff
Copy link
Contributor

@debiff debiff commented Jan 25, 2021

This PR depends on #2746

Short description

This PR add the detail of a Cobadge credit card

List of changes proposed in this pull request

  • Add the Cobadge detail screen
  • Add Cobadge card component

debiff and others added 30 commits January 22, 2021 12:39
@pagopa-github-bot pagopa-github-bot changed the title [#176578587] Cobadge detail screen feat(Bonus Pagamenti Digitali): [#176578587] Cobadge detail screen Jan 25, 2021
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Jan 25, 2021

Affected stories

  • 🌟 #176578587: Come CIT voglio visualizzare la schermata di dettaglio di una carta cobadge

Generated by 🚫 dangerJS against c182524

@codecov
Copy link

codecov bot commented Jan 25, 2021

Codecov Report

Merging #2751 (c182524) into master (9e1a40a) will increase coverage by 0.05%.
The diff coverage is 79.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2751      +/-   ##
==========================================
+ Coverage   53.86%   53.91%   +0.05%     
==========================================
  Files         793      794       +1     
  Lines       21942    21996      +54     
  Branches     4142     4150       +8     
==========================================
+ Hits        11818    11860      +42     
- Misses      10068    10080      +12     
  Partials       56       56              
Impacted Files Coverage Δ
...ures/wallet/cobadge/screen/CobadgeDetailScreen.tsx 65.71% <61.29%> (-34.29%) ⬇️
.../features/wallet/cobadge/component/CobadgeCard.tsx 100.00% <100.00%> (ø)
.../wallet/cobadge/component/CobadgeWalletPreview.tsx 100.00% <100.00%> (ø)

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 9e1a40a...c182524. Read the comment docs.

Comment on lines 32 to 34
width: 80,
height: 40,
resizeMode: "contain"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
width: 80,
height: 40,
resizeMode: "contain"
width: 48,
height: 30,
resizeMode: "contain"

Changed following the Invision specs:
Schermata 2021-01-29 alle 18 18 35

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Modified dimensions accordingly to the InVision specs in 8e95104

Comment on lines 57 to 68
{props.abiLogo && imageStyle ? (
<Image
source={{ uri: props.abiLogo }}
style={imageStyle}
testID={"abiLogo"}
/>
) : (
<Image
source={abiLogoFallback}
style={styles.abiLogoFallback}
testID={"abiLogoFallback"}
/>
Copy link
Contributor

Choose a reason for hiding this comment

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

On iOS I always see the fallback image.

Registrazione.schermo.2021-01-29.alle.18.32.16.mov
Suggested change
{props.abiLogo && imageStyle ? (
<Image
source={{ uri: props.abiLogo }}
style={imageStyle}
testID={"abiLogo"}
/>
) : (
<Image
source={abiLogoFallback}
style={styles.abiLogoFallback}
testID={"abiLogoFallback"}
/>
{props.abiLogo && imageStyle ? (
<Image
source={{ uri: props.abiLogo }}
style={imageStyle}
testID={"abiLogo"}
key={"abiLogo"}
/>
) : (
<Image
source={abiLogoFallback}
style={styles.abiLogoFallback}
testID={"abiLogoFallback"}
key={"abiLogoFallback"}
/>

My guess is that the node occupies the same position and the cache is not refreshered, obtaining something similar to facebook/react-native#9195.
Using different keys it renders correctly for me both on iOS and Android.

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 for the suggestion. Working always on the same card I never notice this bug.
Fixed in a6c4e84

image={getCardIconFromBrandLogo(props.cobadge.info)}
onPress={() => props.navigateToCobadgeDetails(props.cobadge)}
image={brandLogo}
onPress={() => props.navigateToCobadgeDetails(props.cobadge, brandLogo)}
Copy link
Contributor

Choose a reason for hiding this comment

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

What you think about passing only props.cobadge as navigation param? The brandlogo can always be uniquely derived using getCardIconFromBrandLogo.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, sure.
Modified in 0816b1d

const aBrandLogo = defaultCardIcon;
const anExpireMonth = "6";
const anExpireYear = "2021";
describe("BPayWalletPreview component", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
describe("BPayWalletPreview component", () => {
describe("CoBadgeWalletPreview component", () => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 0280969

@fabriziofff fabriziofff merged commit 0192964 into master Feb 2, 2021
@fabriziofff fabriziofff deleted the 176578587-cobadge-detail-screen branch February 2, 2021 11:07
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