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): [#175263506] Create BPD card component #2338

Merged
merged 8 commits into from
Nov 5, 2020

Conversation

CrisTofani
Copy link
Contributor

Short description

This PR adds the base component for BPD card detail

Full card

Preview card

@CrisTofani CrisTofani changed the title 175263506 bpd card component [#175263506] Create BPD card component Nov 3, 2020
@pagopa-github-bot pagopa-github-bot changed the title [#175263506] Create BPD card component feat(Bonus Pagamenti Digitali): [#175263506] Create BPD card component Nov 3, 2020
@pagopa-github-bot
Copy link
Collaborator

pagopa-github-bot commented Nov 3, 2020

Affected stories

  • 🌟 #175263506: Come DEV voglio poter avere un componente generico Card bpd da visualizzare nel wallet

Generated by 🚫 dangerJS against 77dbef7

@codecov-io
Copy link

codecov-io commented Nov 4, 2020

Codecov Report

Merging #2338 into master will decrease coverage by 0.31%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2338      +/-   ##
==========================================
- Coverage   48.68%   48.36%   -0.32%     
==========================================
  Files         615      604      -11     
  Lines       17276    17045     -231     
  Branches     3111     3088      -23     
==========================================
- Hits         8410     8244     -166     
+ Misses       8824     8759      -65     
  Partials       42       42              
Impacted Files Coverage Δ
ts/components/core/typography/H2.tsx 100.00% <ø> (ø)
ts/components/core/typography/H5.tsx 100.00% <ø> (ø)
ts/screens/wallet/WalletHomeScreen.tsx 25.98% <0.00%> (-0.42%) ⬇️
ts/screens/showroom/OthersShowroom.tsx 100.00% <0.00%> (ø)
ts/features/bonus/bpd/model/RemoteValue.ts 52.17% <0.00%> (ø)
ts/features/bonus/bpd/navigation/routes.ts 100.00% <0.00%> (ø)
ts/features/bonus/bpd/navigation/navigator.ts 100.00% <0.00%> (ø)
ts/screens/showroom/core/SelectionShowroom.tsx 100.00% <0.00%> (ø)
ts/features/bonus/bpd/store/actions/onboarding.ts 100.00% <0.00%> (ø)
...s/features/bonus/bpd/screens/test/TMPBpdScreen.tsx 50.00% <0.00%> (ø)
... and 28 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 e79f77e...77dbef7. Read the comment docs.

});

type BadgeDefinition = {
style: any;
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
style: any;
style: StyleProp<ViewStyle>;

Copy link
Contributor

Choose a reason for hiding this comment

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

In order to avoid any

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 wanted to change it before opening the PR, but then missed it at the end. Thanks!

Comment on lines +184 to +190
<Badge style={[formatStatusBadge().style, styles.badgeBase]}>
<Text
semibold={true}
style={styles.badgeTextBase}
dark={props.period.status !== "Closed"}
>
{formatStatusBadge().label}
Copy link
Contributor

Choose a reason for hiding this comment

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

In order to avoid multiple execution of the function formatStatusBadge maybe we can use const badgeFormat = formatStatusBadge() in BpdCardComponent and use badgeFormat.style and badgeFormat.label.
What you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

absolutely! sounds good

@fabriziofff fabriziofff merged commit c9abaef into master Nov 5, 2020
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