Conversation
eileen-nava
requested review from
a team,
aduth and
amirbey
and removed request for
a team
| const isAssessedAsGlare = glare < glareThreshold!; | ||
| const isAssessedAsBlurry = sharpness < sharpnessThreshold!; |
There was a problem hiding this comment.
I'd be hesitant to use the ! operator here if we're building in the expectation that the value may be null.
Instead, maybe we should explicitly handle the scenario that the value is in-fact null, e.g. !!glareThreshold && glare < glareThreshold;
There was a problem hiding this comment.
Yeah, I agree, using the ! operator here made me antsy. I like the approach you suggested and will try it, thanks!
aduth
left a comment
aduth
left a comment
There was a problem hiding this comment.
I haven't actually verified that we do populate these options in all scenarios already, but assuming we do, LGTM 👍
I assume that's what's happening here, and that the configs are never overridden to be explicitly identity-idp/app/views/idv/shared/_document_capture.html.erb Lines 18 to 19 in 43e0c4a |
3 participants
🎫 Ticket
Link to the relevant ticket:
LG-10525
🛠 Summary of changes
My goal was to reduce the confusing discrepancy between the threshold of
50, which we get from the backend, and the front-end constants ofDEFAULT_ACCEPTABLE_GLARE_SCOREandDEFAULT_ACCEPTABLE_SHARPNESS_SCORE, which were set to 30. I wanted to remove those constants, but I am now thinking it may be better to leave them and just change them to 50. Slack context here.