Skip to content

LG-3323 Desktop liveness upload page#4124

Merged
amathews-fs merged 3 commits intomasterfrom
LG-3323-desktop-upload-page-states-amathews
Aug 25, 2020
Merged

LG-3323 Desktop liveness upload page#4124
amathews-fs merged 3 commits intomasterfrom
LG-3323-desktop-upload-page-states-amathews

Conversation

@amathews-fs
Copy link
Copy Markdown
Contributor

Changes to make sure the right states are showing up on the desktop docauth upload page. Added simple camera detection javascript.

…esktop docauth upload page. Added simple camera detection javascript.
<%= render 'idv/doc_auth/start_over_or_cancel' %>
<%= javascript_pack_tag 'image-preview' %>

<% unless Figaro.env.document_capture_react_enabled == 'false' %>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With the unless and the false this is kind of a double negative... WDYT about flipping it?

<% if Figaro.env.document_capture_react_enabled == 'true' %>

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I noticed after I committed. I just stole that from something Doug put up recently. It's weird and I'll flip it.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIRC, originally it wasn't included in application.yml.default, as it was intended for local override. At that time, nil and 'true' were used interchangeably.

See: #3994

That being said, it might also be time to retire this configuration? I think @solipet expected it to be temporary for testing the fallback flow. We can still test this flow by disabling JavaScript (i.e. through Chrome DevTools).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That is also an option. If it was only meant to be temporary I can remove the check. I have been using the browser based "disable javascript" to test anyway.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I just removed the check because it wasn't really relevant anymore.

@amathews-fs amathews-fs marked this pull request as ready for review August 25, 2020 17:31
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -0,0 +1,10 @@
// app/packs/upload-step.js
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

does this line do anything?

@amathews-fs amathews-fs merged commit 25b65d0 into master Aug 25, 2020
@amathews-fs amathews-fs deleted the LG-3323-desktop-upload-page-states-amathews branch August 25, 2020 19:22
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.

3 participants