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

[Wallet] New camera permission flow #1398

Merged
merged 3 commits into from
Oct 18, 2019

Conversation

jeanregisser
Copy link
Contributor

@jeanregisser jeanregisser commented Oct 18, 2019

Description

camera_permissioning

Highlights:

  • react-native-camera now handles the details of asking for permissions
  • a custom view is displayed when the permission has been denied so the
    user can navigate to the settings and allow the permission

Tested

Tested the flow on both iOS and Android.

  • go to qr scanner > deny permission > it shows the message instructing to go to the settings to allow permission > tap 'Settings' > Settings open
  • go to qr scanner > accept permission > it can successfully scan

iOS
IMG_2740
IMG_2737
IMG_2735

Android
iTerm2 NCkoL3
iTerm2 Yn4EU2
iTerm2 uPNfeQ

Other changes

  • You can now tap the QR icon next to "Show your QR code" to trigger the action ;) Previously only tapping the text would trigger it.

Related issues

Backwards compatibility

Yes

- react-native-camera now handles the details of asking for permissions
- a custom view is displayed when the permission has been denied so the
user can navigate to the settings and allow the permission
@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #1398 into master will increase coverage by 0.18%.
The diff coverage is 63.15%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1398      +/-   ##
=========================================
+ Coverage   66.91%   67.1%   +0.18%     
=========================================
  Files         265     266       +1     
  Lines        7780    7776       -4     
  Branches      514     512       -2     
=========================================
+ Hits         5206    5218      +12     
+ Misses       2472    2456      -16     
  Partials      102     102
Flag Coverage Δ
#mobile 67.1% <63.15%> (+0.18%) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/utils/permissions.android.ts 0% <ø> (ø) ⬆️
packages/mobile/src/utils/permissions.ios.ts 0% <ø> (ø) ⬆️
packages/mobile/src/qrcode/QRScanner.tsx 0% <0%> (ø) ⬆️
packages/mobile/src/qrcode/NotAuthorizedView.tsx 85.71% <85.71%> (ø)

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 43c9ebf...51c3e4c. Read the comment docs.

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

One nit but otherwise lgtm

function NotAuthorizedView({ t }: Props) {
const onPressSettings = useCallback(async () => {
if (Platform.OS === 'ios') {
await Linking.openURL('app-settings:')
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use navigateToUri

@jeanregisser jeanregisser added the automerge Have PR merge automatically when checks pass label Oct 18, 2019
@celo-ci-bot-user celo-ci-bot-user merged commit 5d7db19 into master Oct 18, 2019
@celo-ci-bot-user celo-ci-bot-user deleted the jeanregisser/ios-fix-qr-scan branch October 18, 2019 13:48
aaronmgdr added a commit that referenced this pull request Oct 23, 2019
* master: (37 commits)
  [Wallet] Add support for social wallet (Safeguards) import  (#1414)
  [Wallet] Implement new backup flows including social backup (#1399)
  Use validator set precompiles in Attestations (#1248)
  E2E Attestations test + various e2e improvements (#1417)
  Update Footer (#1331)
  [Wallet] New camera permission flow (#1398)
  [Wallet] Enable push notifications on iOS (#1389)
  [Wallet] Add Celo Lite toggle (UI only, zeroSync on/off in other PR) (#1369)
  Document npm inter-repo dependencies instructions (#1370)
  [wallet]Add more documentation on ZeroSync mode (#1367)
  [wallet]Add documentation for jndcrash (#1364)
  Point end-to-end tests back to master (#1372)
  Alfajores changes & comment on unlocking accounts (#1297)
  Reconfigure terraform local configuration during init to allow multiple envs (#773)
  Implement proof-of-stake changes (#1177)
  [Celotool] Update blockchain-api deploy script to automatically update faucet address (#1347)
  Allow most recently reporting oracle to report again (#1288)
  [wallet]Add documentation for ZeroSync mode (#1361)
  Fix Metadata registration during contract deploy (#1346)
  [Wallet] Enable firebase on iOS (#1344)
  ...
aaronmgdr added a commit that referenced this pull request Oct 23, 2019
* master: (128 commits)
  [Wallet] Add support for social wallet (Safeguards) import  (#1414)
  [Wallet] Implement new backup flows including social backup (#1399)
  Use validator set precompiles in Attestations (#1248)
  E2E Attestations test + various e2e improvements (#1417)
  Update Footer (#1331)
  [Wallet] New camera permission flow (#1398)
  [Wallet] Enable push notifications on iOS (#1389)
  [Wallet] Add Celo Lite toggle (UI only, zeroSync on/off in other PR) (#1369)
  Document npm inter-repo dependencies instructions (#1370)
  [wallet]Add more documentation on ZeroSync mode (#1367)
  [wallet]Add documentation for jndcrash (#1364)
  Point end-to-end tests back to master (#1372)
  Alfajores changes & comment on unlocking accounts (#1297)
  Reconfigure terraform local configuration during init to allow multiple envs (#773)
  Implement proof-of-stake changes (#1177)
  [Celotool] Update blockchain-api deploy script to automatically update faucet address (#1347)
  Allow most recently reporting oracle to report again (#1288)
  [wallet]Add documentation for ZeroSync mode (#1361)
  Fix Metadata registration during contract deploy (#1346)
  [Wallet] Enable firebase on iOS (#1344)
  ...
aaronmgdr added a commit that referenced this pull request Oct 23, 2019
* master: (37 commits)
  [Wallet] Add support for social wallet (Safeguards) import  (#1414)
  [Wallet] Implement new backup flows including social backup (#1399)
  Use validator set precompiles in Attestations (#1248)
  E2E Attestations test + various e2e improvements (#1417)
  Update Footer (#1331)
  [Wallet] New camera permission flow (#1398)
  [Wallet] Enable push notifications on iOS (#1389)
  [Wallet] Add Celo Lite toggle (UI only, zeroSync on/off in other PR) (#1369)
  Document npm inter-repo dependencies instructions (#1370)
  [wallet]Add more documentation on ZeroSync mode (#1367)
  [wallet]Add documentation for jndcrash (#1364)
  Point end-to-end tests back to master (#1372)
  Alfajores changes & comment on unlocking accounts (#1297)
  Reconfigure terraform local configuration during init to allow multiple envs (#773)
  Implement proof-of-stake changes (#1177)
  [Celotool] Update blockchain-api deploy script to automatically update faucet address (#1347)
  Allow most recently reporting oracle to report again (#1288)
  [wallet]Add documentation for ZeroSync mode (#1361)
  Fix Metadata registration during contract deploy (#1346)
  [Wallet] Enable firebase on iOS (#1344)
  ...
tkporter pushed a commit that referenced this pull request Oct 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

New camera permission flow
3 participants