LG-11577: selfie UI content update #9746
There are no files selected for viewing
|
dawei-nava marked this conversation as resolved.
Outdated
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| import { t } from '@18f/identity-i18n'; | ||
| import DocumentSideAcuantCapture from './document-side-acuant-capture'; | ||
| import TipList from './tip-list'; | ||
|
|
||
| /** @typedef {import('@18f/identity-form-steps').FormStepError<*>} FormStepError */ | ||
| /** @typedef {import('@18f/identity-form-steps').RegisterFieldCallback} RegisterFieldCallback */ | ||
| /** @typedef {import('@18f/identity-form-steps').OnErrorCallback} OnErrorCallback */ | ||
|
|
||
| /** | ||
| * @typedef DocumentCaptureSelfieCaptureProps | ||
| * | ||
| * @prop {RegisterFieldCallback} registerField | ||
| * @prop {Blob|string|null|undefined} value | ||
| * @prop {(nextValues:{[key:string]: Blob|string|null|undefined})=>void} onChange Update values, | ||
| * merging with existing values. | ||
| * @prop {FormStepError[]} errors | ||
| * @prop {OnErrorCallback} onError | ||
| * @prop {string=} className | ||
| */ | ||
|
|
||
| /** | ||
| * @param {DocumentCaptureSelfieCaptureProps} props Props object. | ||
| */ | ||
| function DocumentCaptureSelfieCapture({ | ||
| registerField, | ||
| value, | ||
| onChange, | ||
| errors, | ||
| onError, | ||
| className, | ||
| }) { | ||
| return ( | ||
| <> | ||
| <hr className="margin-y-5" /> | ||
| <h2>{`2. ${t('doc_auth.headings.document_capture_subheader_selfie')}`}</h2> | ||
|
dawei-nava marked this conversation as resolved.
Outdated
|
||
| <TipList | ||
| titleClassName="margin-bottom-0 text-bold" | ||
| title={t('doc_auth.tips.document_capture_selfie_selfie_text')} | ||
| items={[ | ||
| t('doc_auth.tips.document_capture_selfie_text1'), | ||
| t('doc_auth.tips.document_capture_selfie_text2'), | ||
| t('doc_auth.tips.document_capture_selfie_text3'), | ||
| ]} | ||
| /> | ||
| <DocumentSideAcuantCapture | ||
| key="selfie" | ||
| side="selfie" | ||
| registerField={registerField} | ||
| value={value} | ||
| onChange={onChange} | ||
| errors={errors} | ||
| onError={onError} | ||
| className={className} | ||
| /> | ||
| </> | ||
| ); | ||
| } | ||
|
|
||
| export default DocumentCaptureSelfieCapture; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import TipList from './tip-list'; | |
| import DocumentCaptureNotReady from './document-capture-not-ready'; | ||
| import { FeatureFlagContext } from '../context'; | ||
| import DocumentCaptureAbandon from './document-capture-abandon'; | ||
| import DocumentCaptureSelfieCapture from './document-capture-selfie-capture'; | ||
|
|
||
| /** | ||
| * @typedef {'front'|'back'|'selfie'} DocumentSide | ||
|
|
@@ -47,16 +48,26 @@ function DocumentsStep({ | |
| * | ||
| * @type {DocumentSide[]} | ||
| */ | ||
| const documentSides = ['front', 'back']; | ||
|
Contributor
There was a problem hiding this comment. I see why you went this direction. I agree that we should be splitting things out into components. Instead of doing it this way, I'd recommend you return a component structure like this from this file. Some of my reasoning:
return (
<>
{flowpath === ...}
<PageHeading>...
<DocumentCaptureSubheader> // returns the h2 starting with "1."
<TipList>...
<DocumentFront> // a new component, wraps DocumentSideAcuantCapture
<DocumentBack> // a new component, wraps DocumentSideAcuantCapture
<SelfieCaptureSubheader> // returns the h2 starting with "2."
<Selfie> // a new component, wraps DocumentSideAcuantCapture
{isLastStep ...}
{notReadySectionEnabled ...}
{exisQuestionSectionEnabled ...}
<Cancel>
</>
)And for the const DocumentFront = ({registerField, onChange, onError}: {...new type...}) = {
const side: DocumentSide = 'front'
return (
<DocumentSideAcuantCapture
key={side}
side={side}
registerField={registerField}
value={value[side]}
onChange={onChange}
errors={errors}
onError={onError}
/>
)
}
Contributor
Author
There was a problem hiding this comment. @charleyf , refactored with HOC for document sides. Also I kept selfie part as a separate component, since it contains multiple parts, it feels easier to test as a whole piece.
Contributor
There was a problem hiding this comment.
That makes sense to me. I do still think that everything in A few pieces of evidence:
Another way I'm thinking about this: I can't describe in words what the purpose of
Contributor
Author
There was a problem hiding this comment. Revert how front, back sides rendered, otherwise it cause some document active element issues during testing.
Contributor
Author
There was a problem hiding this comment. @charleyf agree most of the bullet points, and seems backtracked from previously components etc. Also, in general it may be callsed
Contributor
Author
There was a problem hiding this comment. @charleyf , pulled all stuff in document step.
Contributor
There was a problem hiding this comment. Nice! Having all the
I'd be interested to hear more about this. The pseudocode for my suggested component structure with an example component is earlier in this thread. I'm happy to help you implement that, but I think the way the code is currently is close enough.
Contributor
Author
There was a problem hiding this comment. @charleyf , yes basically following is the same with the HOC withProps, where the element focus seems not behaves the same with rendering in places, I too was baffled by it since the change has nothing to do with functionality, but that feels quite elusive and can consume quite some time.
Contributor
There was a problem hiding this comment. ✅ just approved, since I agree it doesn't necessarily make much sense to keep polishing this. Agreed, I'm also quite surprised that extracting those components would change anything. Can you tell me what you mean about the element focus changing? Do you mean the outline or some other aspect? A screenshot would help too.
Contributor
Author
There was a problem hiding this comment. @charleyf in the document-capture-spec test, checks for document.activeElement |
||
| const selfieSide = 'selfie'; | ||
|
|
||
| const pageHeaderText = selfieCaptureEnabled | ||
| ? t('doc_auth.headings.document_capture_with_selfie') | ||
| : t('doc_auth.headings.document_capture'); | ||
|
|
||
| const idTipListTitle = t('doc_auth.tips.document_capture_selfie_id_header_text'); | ||
| return ( | ||
| <> | ||
| {flowPath === 'hybrid' && <HybridDocCaptureWarning className="margin-bottom-4" />} | ||
| <PageHeading>{pageHeaderText}</PageHeading> | ||
| <h2> | ||
| {selfieCaptureEnabled | ||
| ? `1. ${t('doc_auth.headings.document_capture_subheader_id')}` | ||
|
charleyf marked this conversation as resolved.
Outdated
|
||
| : t('doc_auth.headings.document_capture_subheader_id')} | ||
| </h2> | ||
| <TipList | ||
| titleClassName="margin-bottom-0 text-bold" | ||
| title={idTipListTitle} | ||
| items={[ | ||
| t('doc_auth.tips.document_capture_id_text1'), | ||
| t('doc_auth.tips.document_capture_id_text2'), | ||
|
|
@@ -74,6 +85,15 @@ function DocumentsStep({ | |
| onError={onError} | ||
| /> | ||
| ))} | ||
| {selfieCaptureEnabled && ( | ||
| <DocumentCaptureSelfieCapture | ||
| registerField={registerField} | ||
| value={value[selfieSide]} | ||
| onChange={onChange} | ||
| errors={errors} | ||
| onError={onError} | ||
| /> | ||
| )} | ||
| {isLastStep ? <FormStepsButton.Submit /> : <FormStepsButton.Continue />} | ||
| {notReadySectionEnabled && <DocumentCaptureNotReady />} | ||
| {exitQuestionSectionEnabled && <DocumentCaptureAbandon />} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { expect } from 'chai'; | ||
| import { within } from '@testing-library/react'; | ||
| import DocumentCaptureSelfieCapture from '@18f/identity-document-capture/components/document-capture-selfie-capture'; | ||
| import { render } from '../../../support/document-capture'; | ||
|
|
||
| describe('document-capture/components/document-capture-selfie-capture', () => { | ||
| it('renders the form steps', () => { | ||
| const { getAllByRole, getByText } = render( | ||
| <DocumentCaptureSelfieCapture | ||
| value={{}} | ||
| onChange={() => {}} | ||
| errors={[]} | ||
| onError={() => {}} | ||
| registerField={() => undefined} | ||
| />, | ||
| ); | ||
|
|
||
| const header = getByText('2. doc_auth.headings.document_capture_subheader_selfie'); | ||
| expect(header).to.be.ok(); | ||
| const tipListHeader = getByText('doc_auth.tips.document_capture_selfie_selfie_text'); | ||
| expect(tipListHeader).to.be.ok(); | ||
| const lists = getAllByRole('list'); | ||
| const tipList = lists[0]; | ||
| expect(tipList).to.be.ok(); | ||
| const tipListItem = within(tipList).getAllByRole('listitem'); | ||
| tipListItem.forEach((li, idx) => { | ||
| expect(li.textContent).to.equals(`doc_auth.tips.document_capture_selfie_text${idx + 1}`); | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.