-
Notifications
You must be signed in to change notification settings - Fork 106
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): [#175012088] Adds the AddBancomatScreen for Bancomat onboarding #2285
Conversation
Affected stories
|
Codecov Report
@@ Coverage Diff @@
## master #2285 +/- ##
==========================================
- Coverage 47.95% 47.95% -0.01%
==========================================
Files 560 561 +1
Lines 16166 16177 +11
Branches 3221 3221
==========================================
+ Hits 7752 7757 +5
- Misses 8372 8378 +6
Partials 42 42
Continue to review full report at Codecov.
|
locales/en/index.yml
Outdated
addBancomat: | ||
title: "Add bancomat {{current}} of {{length}}" | ||
screenTitle: "Do you want to add this card?" | ||
bodySingular: "Abbiamo trovato 1 carta a te intestata. Procedi con l'aggiunta di questa carta cliccando Aggiungi o clicca Salta per annullare l'operazione." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be in english
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Undermaken Yep, I noticed while developing the following story to connect the bancomat screen with redux, I'm going to fix this here too
locales/it/index.yml
Outdated
screenTitle: "Vuoi aggiungere questa carta?" | ||
bodySingular: "Abbiamo trovato 1 carta a te intestata. Procedi con l'aggiunta di questa carta cliccando Aggiungi o clicca Salta per annullare l'operazione." | ||
bodyPlural: "Abbiamo trovato {{number}} carte a te intestate. Procedi con l'aggiunta di questa carta cliccando Aggiungi o clicca Salta per passare alla carta successiva." | ||
skip: Skip |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be in italian
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the same for the previous comment
ts/features/wallet/onboarding/bancomat/screens/add-pans/AddBancomatScreen.tsx
Show resolved
Hide resolved
@@ -0,0 +1,46 @@ | |||
import { useEffect } from "react"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useImageResize.ts
should be fine
} | ||
}; | ||
|
||
export const useImageResize = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of a callback we can use an internal state and return it.
Could this code be an improvement?
const handleImageDimensionSuccess = (
width: number,
height: number,
maxW: number,
maxH: number
): [number, number] => {
if (width > 0 && height > 0) {
const ratio = Math.min(maxW / width, maxH / height);
return [width * ratio, height * ratio];
}
return [0, 0];
};
export const useImageResize = (
maxWidth: number,
maxHeight: number,
imageUrl?: string
): [number, number] => {
const [size, setSize] = useState<[number, number]>([0, 0]);
useEffect(() => {
fromNullable(imageUrl).map(url =>
Image.getSize(url, (w, h) =>
setSize(handleImageDimensionSuccess(w, h, maxWidth, maxHeight))
)
);
}, [imageUrl]);
return size;
};
usage
const imageDimensions = useImageResize(
BASE_IMG_W,
BASE_IMG_H,
props.bank.logoUrl
);
const imageStyle: StyleProp<ImageStyle> = {
width: imageDimensions[0],
height: imageDimensions[1],
resizeMode: "contain"
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PS I didn't test it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, maybe an external (from the component point of view) state could be useful, I'll give it a try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Undermaken works delightfully, implemented in d6a39b2
Short description
This PR adds the AddBancomatScreen for the Onboarding bancomat flow