[Wallet][ReDesign] QR Code / Scan screens #4017

jeanregisser · 2020-06-05T09:30:57Z

Description

This implements the QR code scan redesign.

Note:

The state when Camera permission is denied is not with a white background like in the design but in dark to simplify the integration with the animation.
Avatar used is the old one.

Other changes

Improve Touchable props types to accept all props from TouchableWithoutFeedback (accessibility, testID, etc)

Tested

Related issues

Epic New design system #3645

Backwards compatibility

Yes.

…orm style See software-mansion/react-native-reanimated#719

…n prompt work nicely with the animation

…ng-redesign

aaronmgdr · 2020-06-05T15:50:46Z

oh wow that slide action! 🤩

aaronmgdr · 2020-06-05T15:53:07Z

packages/mobile/src/navigator/QRNavigator.tsx

+  const [wasFocused, setWasFocused] = useState(isFocused)
+  const [isPartiallyVisible, setIsPartiallyVisible] = useState(false)
+  const cameraPermission = useAsync(check, [
+    Platform.select({ ios: PERMISSIONS.IOS.CAMERA, default: PERMISSIONS.ANDROID.CAMERA }),


why default? and not android?

yes I feel you, TypeScript complains otherwise because Platform.select return type would also contain undefined. Which is not accepted by the check function.

aaronmgdr · 2020-06-05T15:55:43Z

packages/mobile/src/navigator/QRNavigator.tsx

+    [position]
+  )
+
+  const animatedStyle = useMemo(() => {


one thing ive been thinking about is defining big hooks like this outside the component and then just calling them.

at best its more performant (no proof) at worst it keeps the voice in my head that functions should be 10 lines or less feeling less ignored.

yes, kinda torn with this too, I'd say it depends, sometimes it's good to have all the context in a "big" function.
Here the function is still relatively short.

aaronmgdr · 2020-06-05T15:56:34Z

packages/mobile/src/navigator/types.tsx

@@ -9,7 +9,14 @@ import { TransactionDataInput } from 'src/send/SendAmount'
 import { ReviewProps } from 'src/transactions/TransactionReview'
 import { TransferConfirmationCardProps } from 'src/transactions/TransferConfirmationCard'

-// tslint:disable-next-line
+// Typed nested navigator params
+type NestedNavigatorParams<ParamList> = {


Thanks! Had to think carefully about this 🤯 😄

jmrossy

Looks great!! Just a few questions

packages/mobile/src/qrcode/QRGen.tsx

jmrossy · 2020-06-05T15:51:34Z

packages/mobile/src/qrcode/QRScanner.tsx

-    },
-    (qrCode) => qrCode.data
+  const onBarCodeDetected = useCallback(
+    memoize(


Does this work? Or do we create a new memoized function on each render?

yeah im not sure i get why its memoized.

Yes not very obvious on the first look indeed.

It does create a new memoized version on each render, but only the first memoized version is returned, unless [scanIsForSecureSend, transactionData] change.

We could think calling memoize on each render seems heavy, but it's not very different from creating a new function in a standard useCallback call.

See https://github.com/lodash/lodash/blob/master/memoize.js#L43-L60

jmrossy · 2020-06-05T15:53:16Z

packages/mobile/src/qrcode/QRScanner.tsx

+export default function QRScanner({ route }: Props) {
+  const dispatch = useDispatch()
+  const { t } = useTranslation(Namespaces.sendFlow7)
+  const inset = useSafeArea()


I'm wondering if this would work better as an HOC instead of getting the inset and manually using it in margins like we do here.

I'm not sure I get the concern here.
I initially used SafeAreaView, but it was causing a visual glitch in the animation because of its use of onLayout.

jmrossy · 2020-06-05T15:54:39Z

packages/mobile/src/qrcode/QRTabBar.tsx

+  })
+
+  const onPressClose = () => {
+    navigation.dangerouslyGetParent()?.goBack()


Is this method... dangerous?

Good question 😄
From the docs:

Reason why the function is called dangerouslyGetParent is to warn developers against overusing it to eg. get parent of parent and other hard-to-follow patterns.

jmrossy · 2020-06-05T15:55:55Z

packages/mobile/src/send/QRCodeIcon.tsx

@@ -7,7 +7,7 @@ import { Screens } from 'src/navigator/Screens'

 export function QRCodeIcon() {
  const onQrCodePress = React.useCallback(() => {
-    navigate(Screens.QRScanner)
+    navigate(Screens.QRNavigator, { screen: Screens.QRScanner })


Is screen the default screen here? If so, maybe rename it?

No, default screen is QRCode

jmrossy · 2020-06-05T15:56:38Z

packages/react-components/components/BorderlessButton.v2.tsx

 }

 // unstyled Touchable Text, good for making other Text Buttons such as TopBarButton
 export default function BoarderlessButton(props: Props) {
-  const { onPress, style, children, disabled, testID } = props
+  const { style, children, ...rest } = props


Nit: I believe passThroughProps is a common convention for rest here

Agreed 👍

jmrossy · 2020-06-05T15:58:53Z

patches/react-native-tab-view+2.14.4.patch

@@ -0,0 +1,36 @@
+patch-package


Is there a github issue open on the repo with this issue? If not, should we open one? Or use a diff lib we don't have to patch?

Very good concern, cut a few corners to avoid spending too much time on this here.
I plan to open a PR on react-native-tab-view.
The lib itself is very solid, I've used it in a bunch of past projects and a lot of effort has been put into it to provide native feeling for this.
It was created by @satya164 which is also one of the creator of of react-navigation.

…ng-redesign

jeanregisser added 22 commits June 2, 2020 15:47

[WiP] Proof of concept

1d92b05

Animate colors within the tab bar from light to dark

725dc80

Upgrade react-native-reanimated to fix TypeScript issue within transf…

2ec382a

…orm style See software-mansion/react-native-reanimated#719

Fix types

d5f778b

Fix scan

379b3a5

Style unauthorized view

00eecce

Activate the camera only when necessary and make first time permissio…

54d0e9c

…n prompt work nicely with the animation

Patch pager and use appropriate one based on the platform

7eb23a2

Clean up code

2e1c610

Fix navigation to nested navigators, with type safe params

a1a5bdd

Better type for when the params are undefined

655f2f2

Merge remote-tracking branch 'origin/master' into jeanregisser/scanni…

e794982

…ng-redesign

Clean up SegmentedControl

c5d71ad

Clean up interpolateColors

8a9288a

Update tests

3c8739c

Update translations

6b7dc67

Clean up QRTabBar

79e20ac

Tweak status bar style on Android

cca7b20

Fix tabs not touchable on Android

d6f1951

Add test for SegmentedControl

d9c800c

Update require

7487eae

Merge remote-tracking branch 'origin/master' into jeanregisser/scanni…

2e7c7a3

…ng-redesign

jeanregisser marked this pull request as ready for review June 5, 2020 13:14

jeanregisser requested review from aaronmgdr, asaj, cmcewen, jmrossy and timmoreton as code owners June 5, 2020 13:14

jeanregisser requested a review from a team June 5, 2020 13:16

i1skn mentioned this pull request Jun 5, 2020

New design system #3645

Closed

50 tasks

aaronmgdr reviewed Jun 5, 2020

View reviewed changes

jmrossy reviewed Jun 5, 2020

View reviewed changes

jeanregisser added 3 commits June 8, 2020 15:22

Rename rest -> passthroughProps

f1088ed

Merge remote-tracking branch 'origin/master' into jeanregisser/scanni…

58d8411

…ng-redesign

Remove default value

0f59953

jmrossy approved these changes Jun 8, 2020

View reviewed changes

Merge remote-tracking branch 'origin/master' into jeanregisser/scanni…

98fbc34

…ng-redesign

jeanregisser added the automerge Have PR merge automatically when checks pass label Jun 9, 2020

jeanregisser merged commit 093734d into master Jun 9, 2020

jeanregisser deleted the jeanregisser/scanning-redesign branch June 9, 2020 09:45

tarikbellamine mentioned this pull request Jun 13, 2020

User is able to get back from QR code screen when tapping on top left corner even Back icon is not shown #4048

Closed

snyk-bot mentioned this pull request Jan 21, 2022

[Snyk] Security upgrade firebase-tools from 7.16.2 to 10.1.2 leojoy95/celo-monorepo#238

Open

leojoy95 mentioned this pull request Mar 22, 2024

[Snyk] Fix for 1 vulnerabilities leojoy95/celo-monorepo#888

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Wallet][ReDesign] QR Code / Scan screens #4017

[Wallet][ReDesign] QR Code / Scan screens #4017

jeanregisser commented Jun 5, 2020 •

edited

Loading

aaronmgdr commented Jun 5, 2020

aaronmgdr Jun 5, 2020

jeanregisser Jun 5, 2020

aaronmgdr Jun 5, 2020

aaronmgdr Jun 5, 2020

jeanregisser Jun 8, 2020 •

edited

Loading

aaronmgdr Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy left a comment

jmrossy Jun 5, 2020

aaronmgdr Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy Jun 5, 2020

jeanregisser Jun 8, 2020

jmrossy Jun 5, 2020

jeanregisser Jun 8, 2020 •

edited

Loading

[Wallet][ReDesign] QR Code / Scan screens #4017

[Wallet][ReDesign] QR Code / Scan screens #4017

Conversation

jeanregisser commented Jun 5, 2020 • edited Loading

Description

Other changes

Tested

Related issues

Backwards compatibility

aaronmgdr commented Jun 5, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeanregisser Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmrossy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeanregisser Jun 8, 2020 • edited Loading

Choose a reason for hiding this comment

jeanregisser commented Jun 5, 2020 •

edited

Loading

jeanregisser Jun 8, 2020 •

edited

Loading

jeanregisser Jun 8, 2020 •

edited

Loading