chore(liveness): allow selecting all cameras, allow camera selection … #5833

thaddmt · 2024-09-24T19:08:18Z

…on mobile

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

Have read the Pull Request Guidelines
PR description included
yarn test passes and tests are updated/added
PR title and commit messages follow conventional commit syntax
If this change should result in a version bump, changeset added (This can be done after creating the PR.) This does not apply to changes made to docs, e2e, examples, or other private packages.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…on mobile

changeset-bot · 2024-09-24T19:08:22Z

⚠️ No Changeset found

Latest commit: c76bf05

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

reesscot · 2024-09-25T00:30:43Z

...es/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx

@@ -115,6 +115,8 @@ export const LivenessCameraModule = (

  const isFaceMovementChallenge =
    useLivenessSelector(selectChallengeType) === FACE_MOVEMENT_CHALLENGE.type;
+  const isMobileFaceMovementAndLightChallenge =


Can we flip this logic to be more readable, something like
const allowSelectableDevices = !isMobile || (isMobile && isFaceMovementChallenge)
then add a comment why we only support desktop or mobile and facemovement?

esauerbo · 2024-09-25T16:47:26Z

packages/e2e/features/ui/components/liveness/disable-start-screen.feature

@@ -24,6 +24,3 @@ Feature: Disable Start Screen
      Then I click the "FaceMovementAndLightChallenge" selectfield and select the "FaceMovementChallenge" option
      Then I see "FaceMovementChallenge"
      Then I see "liveness-detector" element
-      Then I see the "Face didn't fit inside oval in time limit." timeout error


Why do these need to be removed?

Rekognition updated the values for face distance which make it so these tests no longer work, only on the no light stuff, figure that's unrelated to this PR but just want to unblock this

Oh gotcha, what updates would we need to get them passing? But make sense as a follow up

We would need to update the video we use in the e2e test haha

reesscot · 2024-09-25T18:05:44Z

...es/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx

@@ -115,6 +115,8 @@

  const isFaceMovementChallenge =
    useLivenessSelector(selectChallengeType) === FACE_MOVEMENT_CHALLENGE.type;
+  const allowSelectableDevicesMobile =
+    !isMobileScreen || (isMobileScreen && isFaceMovementChallenge);


Why does this think isMobileScreen is always true?

I believe because if isMobileScreen === false then the left hand side will always short circuit the whole thing to be true. So you only evaluate the right side if isMobileScreen === true so it's redundant

boolean algebra 🙃

reesscot · 2024-09-25T22:36:22Z

packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/CameraSelector.tsx

+  selectableDevices: MediaDeviceInfo[];
+}
+
+export const CameraSelector = (props: CameraSelectorProps): JSX.Element => {


reesscot · 2024-09-25T22:37:55Z

packages/react-liveness/src/components/FaceLivenessDetector/utils/helpers.ts

@@ -8,5 +8,4 @@ export const STATIC_VIDEO_CONSTRAINTS: MediaTrackConstraints = {
    ideal: 480,
  },
  frameRate: { min: 15, ideal: 30, max: 30 },
-  facingMode: 'user',


Just to confirm, this is no longer needed because with the light challenge we only show the dropdown on desktop, and on desktop the facing mode is not relevant?

Exactly, desktop cameras do not have a facingMode value

calebpollman

Minor feedback, lmk your thoughts!

calebpollman · 2024-09-26T00:15:40Z

packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/CameraSelector.tsx

+          value={selectedDeviceId}
+          onChange={onCameraChange}
+        >
+          {selectableDevices?.map((device) => (


Since we are gating rendering of this component think we can remove the optional chaining here

Good catch, removed

...es/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx

calebpollman · 2024-09-26T00:25:15Z

...es/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx

+          {allowSelectableDevices && (
+            <CameraSelector
+              onCameraChange={onCameraChange}
+              selectableDevices={selectableDevices}
+              selectedDeviceId={selectedDeviceId}
+            />
+          )}


Prefer ternary returning null as the second condition in place of returning false

calebpollman · 2024-09-26T00:29:11Z

...es/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx

@@ -160,6 +153,15 @@ export const LivenessCameraModule = (
    videoWidth && videoHeight ? videoWidth / videoHeight : 0
  );

+  // Only allow on mobile screen if it is no light challenge
+  const allowSelectableDevicesMobile =
+    !isMobileScreen || isFaceMovementChallenge;


It's a bit hard to follow that allowSelectableDevicesMobile is true when isMobileScreen is false. Realize that it going to be hard for this not to be somewhat confusing but maybe we can update the approach to something like:

const hasMultipleDevices = !!selectableDevices?.length; const allowDeviceSelection = (isStartView && hasMultipleDevices) && (!isMobileScreen || isFaceMovementChallenge)

Think that makes sense, think this is much cleaner

calebpollman · 2024-09-26T00:32:14Z

packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/CameraSelector.tsx

+  selectedDeviceId?: string;
+  onCameraChange: (e: React.ChangeEvent<HTMLSelectElement>) => void;
+  selectableDevices: MediaDeviceInfo[];


IMO we can simplify the naming of the props a bit here since camera selection is implied by the component name, e.g.

deviceId?: string; onSelect?: (e: React.ChangeEvent<HTMLSelectElement>) => void; devices: MediaDeviceInfo[];

on board with this

…venessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com>

commit 984fa99 Author: thaddmt <68032955+thaddmt@users.noreply.github.com> Date: Thu Sep 26 11:10:45 2024 -0700 chore(liveness): allow selecting all cameras, allow camera selection … (aws-amplify#5833) * chore(liveness): allow selecting all cameras, allow camera selection on mobile * only show on mobile with facemovement challenge only * relax e2e tests on alpha * make more reaadable * fix * update names * refactor, add tests * Update packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com> * refactor --------- Co-authored-by: Caleb Pollman <cpollman@amazon.com>

* Update publish workflow * disable e2e tests in workflow * Squashed commit of the following: commit 984fa99 Author: thaddmt <68032955+thaddmt@users.noreply.github.com> Date: Thu Sep 26 11:10:45 2024 -0700 chore(liveness): allow selecting all cameras, allow camera selection … (#5833) * chore(liveness): allow selecting all cameras, allow camera selection on mobile * only show on mobile with facemovement challenge only * relax e2e tests on alpha * make more reaadable * fix * update names * refactor, add tests * Update packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com> * refactor --------- Co-authored-by: Caleb Pollman <cpollman@amazon.com>

* chore(liveness): allow selecting all cameras, allow camera selection … (#5833) * chore(liveness): allow selecting all cameras, allow camera selection on mobile * only show on mobile with facemovement challenge only * relax e2e tests on alpha * make more reaadable * fix * update names * refactor, add tests * Update packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com> * refactor --------- Co-authored-by: Caleb Pollman <cpollman@amazon.com> * fix(liveness): fix camera select showing up with one camera, fix camera changing weirdness with hair check screen (#5845) --------- Co-authored-by: thaddmt <68032955+thaddmt@users.noreply.github.com> Co-authored-by: Caleb Pollman <cpollman@amazon.com>

* chore(liveness): allow selecting all cameras, allow camera selection … (#5833) * chore(liveness): allow selecting all cameras, allow camera selection on mobile * fix(liveness): fix camera select showing up with one camera, fix camera changing weirdness with hair check screen (#5845) * fix(liveness): Only apply transform style on user-facing video (#5953) * fix(liveness): Fix oval render when switching cameras (#5954) --------- Co-authored-by: thaddmt <68032955+thaddmt@users.noreply.github.com> Co-authored-by: Caleb Pollman <cpollman@amazon.com> Co-authored-by: Emma Sauerborn <70536670+esauerbo@users.noreply.github.com> Co-authored-by: Scott Rees <6165315+reesscot@users.noreply.github.com>

* chore(liveness): allow selecting all cameras, allow camera selection … (#5833) * chore(liveness): allow selecting all cameras, allow camera selection on mobile * only show on mobile with facemovement challenge only * relax e2e tests on alpha * make more reaadable * fix * update names * refactor, add tests * Update packages/react-liveness/src/components/FaceLivenessDetector/LivenessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com> * refactor --------- Co-authored-by: Caleb Pollman <cpollman@amazon.com> * fix(liveness): fix camera select showing up with one camera, fix camera changing weirdness with hair check screen (#5845) * fix(liveness): Only apply transform style on user-facing video (#5953) Co-authored-by: Scott Rees <6165315+reesscot@users.noreply.github.com> * fix(liveness): Fix oval render when switching cameras (#5954) * chore(liveness): update video constraints for selected camera (#6009) --------- Co-authored-by: thaddmt <68032955+thaddmt@users.noreply.github.com> Co-authored-by: Caleb Pollman <cpollman@amazon.com> Co-authored-by: Emma Sauerborn <70536670+esauerbo@users.noreply.github.com> Co-authored-by: Scott Rees <6165315+reesscot@users.noreply.github.com>

chore(liveness): allow selecting all cameras, allow camera selection …

c26511f

…on mobile

thaddmt requested a review from a team as a code owner September 24, 2024 19:08

thaddmt temporarily deployed to ci September 24, 2024 19:08 — with GitHub Actions Inactive

thaddmt temporarily deployed to ci September 24, 2024 19:18 — with GitHub Actions Inactive

thaddmt had a problem deploying to ci September 24, 2024 19:18 — with GitHub Actions Failure

thaddmt temporarily deployed to ci September 24, 2024 19:18 — with GitHub Actions Inactive

thaddmt had a problem deploying to ci September 24, 2024 20:21 — with GitHub Actions Failure

only show on mobile with facemovement challenge only

60c11ce

thaddmt temporarily deployed to ci September 24, 2024 21:34 — with GitHub Actions Inactive

thaddmt temporarily deployed to ci September 24, 2024 21:42 — with GitHub Actions Inactive

thaddmt had a problem deploying to ci September 24, 2024 21:42 — with GitHub Actions Failure

thaddmt temporarily deployed to ci September 24, 2024 21:42 — with GitHub Actions Inactive

relax e2e tests on alpha

e4a7464

thaddmt temporarily deployed to ci September 24, 2024 22:23 — with GitHub Actions Inactive

thaddmt temporarily deployed to ci September 24, 2024 22:31 — with GitHub Actions Inactive

reesscot reviewed Sep 25, 2024

View reviewed changes

esauerbo reviewed Sep 25, 2024

View reviewed changes

make more reaadable

077bb8e

thaddmt requested review from reesscot and esauerbo September 25, 2024 17:12

thaddmt temporarily deployed to ci September 25, 2024 17:12 — with GitHub Actions Inactive

github-advanced-security bot found potential problems Sep 25, 2024

View reviewed changes

thaddmt temporarily deployed to ci September 25, 2024 17:23 — with GitHub Actions Inactive

thaddmt requested a review from calebpollman September 25, 2024 21:45

reesscot reviewed Sep 25, 2024

View reviewed changes

reesscot previously approved these changes Sep 25, 2024

View reviewed changes

calebpollman reviewed Sep 26, 2024

View reviewed changes

Update packages/react-liveness/src/components/FaceLivenessDetector/Li…

6099462

…venessCheck/LivenessCameraModule.tsx Co-authored-by: Caleb Pollman <cpollman@amazon.com>

thaddmt dismissed reesscot’s stale review via 6099462 September 26, 2024 17:01

thaddmt temporarily deployed to ci September 26, 2024 17:01 — with GitHub Actions Inactive

refactor

c76bf05

thaddmt temporarily deployed to ci September 26, 2024 17:08 — with GitHub Actions Inactive

calebpollman approved these changes Sep 26, 2024

View reviewed changes

thaddmt requested review from calebpollman and reesscot September 26, 2024 17:14

thaddmt temporarily deployed to ci September 26, 2024 17:18 — with GitHub Actions Inactive

reesscot approved these changes Sep 26, 2024

View reviewed changes

calebpollman approved these changes Sep 26, 2024

View reviewed changes

thaddmt merged commit 984fa99 into liveness/alpha Sep 26, 2024
32 checks passed

thaddmt deleted the liveness-camera-select-mobile branch September 26, 2024 18:10

ayush987goyal mentioned this pull request Sep 28, 2024

Merge liveness/alpha into dca v2 #5850

Merged

5 tasks

ayush987goyal mentioned this pull request Nov 1, 2024

Merge liveness/alpha into liveness dca v2 #5992

Merged

5 tasks

ayush987goyal mentioned this pull request Nov 6, 2024

liveness dca v2 #6016

Merged

5 tasks

ayush987goyal mentioned this pull request Nov 7, 2024

liveness dca v2: Update selecter userCamera metadata #6024

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(liveness): allow selecting all cameras, allow camera selection … #5833

chore(liveness): allow selecting all cameras, allow camera selection … #5833

thaddmt commented Sep 24, 2024

changeset-bot bot commented Sep 24, 2024 •

edited

Loading

reesscot Sep 25, 2024

esauerbo Sep 25, 2024

thaddmt Sep 25, 2024

esauerbo Sep 25, 2024

thaddmt Sep 25, 2024

reesscot Sep 25, 2024

thaddmt Sep 25, 2024

thaddmt Sep 25, 2024

reesscot Sep 25, 2024

reesscot Sep 25, 2024 •

edited

Loading

thaddmt Sep 25, 2024

calebpollman left a comment

calebpollman Sep 26, 2024

thaddmt Sep 26, 2024

calebpollman Sep 26, 2024

thaddmt Sep 26, 2024

calebpollman Sep 26, 2024

thaddmt Sep 26, 2024

calebpollman Sep 26, 2024

thaddmt Sep 26, 2024

chore(liveness): allow selecting all cameras, allow camera selection … #5833

chore(liveness): allow selecting all cameras, allow camera selection … #5833

Conversation

thaddmt commented Sep 24, 2024

Description of changes

Issue #, if available

Description of how you validated changes

Checklist

changeset-bot bot commented Sep 24, 2024 • edited Loading

⚠️ No Changeset found

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

reesscot Sep 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

calebpollman 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

changeset-bot bot commented Sep 24, 2024 •

edited

Loading

reesscot Sep 25, 2024 •

edited

Loading