-
Notifications
You must be signed in to change notification settings - Fork 4
feat: ui flexibility prototype version 1 #766
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,7 @@ import { UnprocessableEntityErrorObject } from '@gusto/embedded-api/models/error | |
| import { QueryErrorResetBoundary } from '@tanstack/react-query' | ||
| import type { EntityErrorObject } from '@gusto/embedded-api/models/components/entityerrorobject' | ||
| import { FadeIn } from '../Common/FadeIn/FadeIn' | ||
| import { BaseContext, type KnownErrors, type OnEventType } from './useBase' | ||
| import { BaseContext, useBase, type KnownErrors, type OnEventType } from './useBase' | ||
| import { componentEvents, type EventType } from '@/shared/constants' | ||
| import { InternalError } from '@/components/Common' | ||
| import { useComponentContext } from '@/contexts/ComponentAdapter/useComponentContext' | ||
|
|
@@ -36,17 +36,15 @@ export interface BaseComponentInterface<TResourceKey extends keyof Resources = k | |
|
|
||
| type SubmitHandler<T> = (data: T) => Promise<void> | ||
|
|
||
| export const BaseComponent = <TResourceKey extends keyof Resources = keyof Resources>({ | ||
| export const BaseComponentProvider = <TResourceKey extends keyof Resources = keyof Resources>({ | ||
| children, | ||
| FallbackComponent = InternalError, | ||
| LoaderComponent: LoadingIndicatorFromProps, | ||
| onEvent, | ||
| onEvent = () => {}, | ||
| }: BaseComponentInterface<TResourceKey>) => { | ||
| const [error, setError] = useState<KnownErrors | null>(null) | ||
| const [fieldErrors, setFieldErrors] = useState<EntityErrorObject[] | null>(null) | ||
| const throwError = useAsyncError() | ||
| const { t } = useTranslation() | ||
| const Components = useComponentContext() | ||
|
|
||
| const { LoadingIndicator: LoadingIndicatorFromContext } = useLoadingIndicator() | ||
|
|
||
|
|
@@ -90,6 +88,7 @@ export const BaseComponent = <TResourceKey extends keyof Resources = keyof Resou | |
| return ( | ||
| <BaseContext.Provider | ||
| value={{ | ||
| error, | ||
| fieldErrors, | ||
| setError: setErrorWithFieldsClear, | ||
| onEvent, | ||
|
|
@@ -107,23 +106,46 @@ export const BaseComponent = <TResourceKey extends keyof Resources = keyof Resou | |
| onEvent(componentEvents.ERROR, err) | ||
| }} | ||
| > | ||
| {(error || fieldErrors) && ( | ||
| <Components.Alert label={t('status.errorEncountered')} status="error"> | ||
| {fieldErrors && <Components.UnorderedList items={renderErrorList(fieldErrors)} />} | ||
| {error && error instanceof APIError && ( | ||
| <Components.Text>{error.message}</Components.Text> | ||
| )} | ||
| {error && error instanceof SDKValidationError && ( | ||
| <Components.Text as="pre">{error.pretty()}</Components.Text> | ||
| )} | ||
| </Components.Alert> | ||
| )} | ||
| <Suspense fallback={<LoaderComponent />}> | ||
| <FadeIn>{children}</FadeIn> | ||
| </Suspense> | ||
| <Suspense fallback={<LoaderComponent />}>{children}</Suspense> | ||
| </ErrorBoundary> | ||
| )} | ||
| </QueryErrorResetBoundary> | ||
| </BaseContext.Provider> | ||
| ) | ||
| } | ||
|
|
||
| export const BaseUIComponent = ({ children }: { children: ReactNode }) => { | ||
| const { error, fieldErrors } = useBase() | ||
| const { t } = useTranslation() | ||
| const Components = useComponentContext() | ||
|
|
||
| return ( | ||
| <FadeIn> | ||
| {(error || fieldErrors) && ( | ||
| <Components.Alert label={t('status.errorEncountered')} status="error"> | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm just leaving notes here as I go so MAYBE i'm missing something. But have we thought about how they might re-locate or reposition this inside their app? |
||
| {fieldErrors && <Components.UnorderedList items={renderErrorList(fieldErrors)} />} | ||
| {error && error instanceof APIError && <Components.Text>{error.message}</Components.Text>} | ||
| {error && error instanceof SDKValidationError && ( | ||
| <Components.Text as="pre">{error.pretty()}</Components.Text> | ||
| )} | ||
| </Components.Alert> | ||
| )} | ||
| {children} | ||
| </FadeIn> | ||
| ) | ||
| } | ||
|
|
||
| // Existing BaseComponent composes BaseComponentProvider and BaseUIComponent | ||
| // These were separated to allow the provider functionality to be used without | ||
| // importing the UI components. BaseUIComponent can be used for our codebase | ||
| // UI implementations for rendering the errors in a single place. | ||
| export const BaseComponent = <TResourceKey extends keyof Resources = keyof Resources>({ | ||
| children, | ||
| ...props | ||
| }: BaseComponentInterface<TResourceKey>) => { | ||
| return ( | ||
| <BaseComponentProvider {...props}> | ||
| <BaseUIComponent>{children}</BaseUIComponent> | ||
| </BaseComponentProvider> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,202 @@ | ||
| import { beforeEach, describe, expect, it, vi } from 'vitest' | ||
| import { screen } from '@testing-library/react' | ||
| import userEvent from '@testing-library/user-event' | ||
| import { HttpResponse } from 'msw' | ||
| import Address from './ContractorAddressForm' | ||
| import { server } from '@/test/mocks/server' | ||
| import { | ||
| handleGetContractor, | ||
| handleGetContractorAddress, | ||
| handleUpdateContractorAddress, | ||
| } from '@/test/mocks/apis/contractor_address' | ||
| import { setupApiTestMocks } from '@/test/mocks/apiServer' | ||
| import { contractorEvents } from '@/shared/constants' | ||
| import { renderWithProviders } from '@/test-utils/renderWithProviders' | ||
|
|
||
| describe('Contractor/Address', () => { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file just has the original unit test copied over to test for regressions |
||
| beforeEach(() => { | ||
| setupApiTestMocks() | ||
| }) | ||
|
|
||
| describe('when API has minimal address details', () => { | ||
| const exampleUpdatedAddress = { | ||
| version: 'contractor-address-version-updated', | ||
| country: 'USA', | ||
| street_1: '123 Main St', | ||
| street_2: 'Apt 4B', | ||
| city: 'Denver', | ||
| state: 'CO', | ||
| zip: '80202', | ||
| } | ||
|
|
||
| beforeEach(() => { | ||
| server.use( | ||
| handleGetContractorAddress(() => | ||
| HttpResponse.json({ | ||
| version: 'contractor-address-version', | ||
| street_1: null, | ||
| street_2: null, | ||
| city: null, | ||
| state: null, | ||
| zip: null, | ||
| country: 'USA', | ||
| }), | ||
| ), | ||
| handleUpdateContractorAddress(() => HttpResponse.json(exampleUpdatedAddress)), | ||
| ) | ||
| }) | ||
|
|
||
| it('should allow submitting the form', async () => { | ||
| const user = userEvent.setup() | ||
| const mockOnEvent = vi.fn() | ||
|
|
||
| renderWithProviders(<Address contractorId="contractor_id" onEvent={mockOnEvent} />) | ||
|
|
||
| await screen.findByText('Home address') | ||
|
|
||
| await user.type(screen.getByLabelText('Street 1'), '123 Main St') | ||
| await user.type(screen.getByLabelText(/Street 2/i), 'Apt 4B') | ||
| await user.type(screen.getByLabelText('City'), 'Denver') | ||
|
|
||
| const stateControl = screen.getByRole('button', { | ||
| name: /Select state.../i, | ||
| expanded: false, | ||
| }) | ||
| await user.click(stateControl) | ||
| const coloradoOption = screen.getByRole('option', { | ||
| name: /Colorado/i, | ||
| }) | ||
| await user.click(coloradoOption) | ||
|
|
||
| await user.type(screen.getByLabelText('Zip'), '80202') | ||
|
|
||
| const continueButton = screen.getByRole('button', { | ||
| name: /Continue/i, | ||
| }) | ||
| await user.click(continueButton) | ||
|
|
||
| expect(mockOnEvent).toHaveBeenNthCalledWith( | ||
| 1, | ||
| contractorEvents.CONTRACTOR_ADDRESS_UPDATED, | ||
| expect.objectContaining({ | ||
| version: exampleUpdatedAddress.version, | ||
| street1: exampleUpdatedAddress.street_1, | ||
| street2: exampleUpdatedAddress.street_2, | ||
| city: exampleUpdatedAddress.city, | ||
| state: exampleUpdatedAddress.state, | ||
| zip: exampleUpdatedAddress.zip, | ||
| country: exampleUpdatedAddress.country, | ||
| }), | ||
| ) | ||
|
|
||
| expect(mockOnEvent).toHaveBeenNthCalledWith(2, contractorEvents.CONTRACTOR_ADDRESS_DONE) | ||
| }) | ||
|
|
||
| it('should allow setting default values', async () => { | ||
| renderWithProviders( | ||
| <Address | ||
| contractorId="contractor_id" | ||
| onEvent={() => {}} | ||
| defaultValues={{ | ||
| street1: '999 Default St', | ||
| street2: 'Apt 123', | ||
| city: 'Default City', | ||
| state: 'CO', | ||
| zip: '80202', | ||
| }} | ||
| />, | ||
| ) | ||
|
|
||
| await screen.findByText('Home address') | ||
|
|
||
| expect(screen.getByLabelText('Street 1')).toHaveValue('999 Default St') | ||
| expect(screen.getByLabelText(/Street 2/i)).toHaveValue('Apt 123') | ||
| expect(screen.getByLabelText('City')).toHaveValue('Default City') | ||
| expect( | ||
| screen.getByRole('button', { | ||
| name: /Colorado/i, | ||
| expanded: false, | ||
| }), | ||
| ).toBeInTheDocument() | ||
| expect(screen.getByLabelText('Zip')).toHaveValue('80202') | ||
| }) | ||
| }) | ||
|
|
||
| describe('when API has full address details', () => { | ||
| beforeEach(() => { | ||
| server.use( | ||
| handleGetContractorAddress(() => | ||
| HttpResponse.json({ | ||
| version: 'contractor-address-version', | ||
| street_1: '999 Kiera Stravenue', | ||
| street_2: 'Suite 541', | ||
| city: 'San Francisco', | ||
| state: 'CA', | ||
| zip: '94107', | ||
| country: 'USA', | ||
| }), | ||
| ), | ||
| ) | ||
| }) | ||
|
|
||
| it('should defer to values from API over default values', async () => { | ||
| renderWithProviders( | ||
| <Address | ||
| contractorId="contractor_id" | ||
| onEvent={() => {}} | ||
| defaultValues={{ | ||
| street1: '999 Default St', | ||
| street2: 'Apt 123', | ||
| city: 'Default City', | ||
| state: 'CO', | ||
| zip: '80202', | ||
| }} | ||
| />, | ||
| ) | ||
|
|
||
| await screen.findByText('Home address') | ||
|
|
||
| expect(screen.getByLabelText('Street 1')).toHaveValue('999 Kiera Stravenue') | ||
| expect(screen.getByLabelText(/Street 2/i)).toHaveValue('Suite 541') | ||
| expect(screen.getByLabelText('City')).toHaveValue('San Francisco') | ||
| expect( | ||
| screen.getByRole('button', { | ||
| name: /California/i, | ||
| expanded: false, | ||
| }), | ||
| ).toBeInTheDocument() | ||
| expect(screen.getByLabelText('Zip')).toHaveValue('94107') | ||
| }) | ||
| }) | ||
|
|
||
| describe('contractor type in heading', () => { | ||
| it('should show individual text when contractorType is Individual', async () => { | ||
| renderWithProviders(<Address contractorId="contractor_id" onEvent={() => {}} />) | ||
|
|
||
| expect(await screen.findByText('Home address')).toBeInTheDocument() | ||
| expect( | ||
| screen.getByText("Contractor's home mailing address, within the United States."), | ||
| ).toBeInTheDocument() | ||
| }) | ||
|
|
||
| it('should show business text when contractorType is Business', async () => { | ||
| server.use( | ||
| handleGetContractor(() => { | ||
| return HttpResponse.json({ | ||
| uuid: 'contractor_id', | ||
| type: 'Business', | ||
| is_active: true, | ||
| file_new_hire_report: false, | ||
| }) | ||
| }), | ||
| ) | ||
|
|
||
| renderWithProviders(<Address contractorId="contractor_id" onEvent={() => {}} />) | ||
|
|
||
| expect(await screen.findByText('Business address')).toBeInTheDocument() | ||
| expect( | ||
| screen.getByText("Contractor's business address, within the United States."), | ||
| ).toBeInTheDocument() | ||
| }) | ||
| }) | ||
| }) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We provide both the error and fieldErrors via context. They only get our UI alerts if they use the BaseUIComponent below.
Still need to figure out how to navigate this with fieldErrors that need to be applied to the form fields. Do we just leave it to the partner to wire those up? Or do we find a way to normalize all form related errors so they aren't getting them from both the validation schema and the fieldErrors
Additionally, we need to investigate more how to help this be more ergonomic so they're not forced to respect both error and fieldErrors