Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions packages/core/src/app/address/AddressForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -153,34 +153,39 @@ describe('AddressForm Component', () => {
expect(onChange).toHaveBeenCalledWith(fieldId, fieldValue);
});

describe('new phone number validation experiment', () => {
describe('phone number validation setting', () => {
const phoneFormFieldMock = {
fieldType: 'text',
id: 'phone',
name: 'phone',
} as FormField;

const getConfigMockWithPhoneExperimentTrue = (
providerWithCustomCheckout: string | null = null,
) => {
const getConfigMockWithPhoneValidation = (isPhoneNumberValidationEnabled: boolean) => {
const config = getStoreConfig();

return {
...config,
checkoutSettings: {
...config.checkoutSettings,
features: {
...config.checkoutSettings.features,
'CHECKOUT-9019.use_new_phone_number_validation': true,
},
providerWithCustomCheckout,
isPhoneNumberValidationEnabled,
},
};
};

it('renders legacy phone field when PayPal Fastlane powers custom checkout', () => {
it('renders new phone number field when isPhoneNumberValidationEnabled is true', () => {
jest.spyOn(checkoutService.getState().data, 'getConfig').mockReturnValue(
getConfigMockWithPhoneExperimentTrue('bigcommerce_payments_fastlane'),
getConfigMockWithPhoneValidation(true),
);

renderAddressFormComponent({ formFields: [...formFields, phoneFormFieldMock] });

expect(screen.getByTestId('intl-tel-input-mock')).toBeInTheDocument();
expect(screen.queryByTestId('phoneInput-text')).not.toBeInTheDocument();
});

it('renders legacy phone field when isPhoneNumberValidationEnabled is false', () => {
jest.spyOn(checkoutService.getState().data, 'getConfig').mockReturnValue(
getConfigMockWithPhoneValidation(false),
);

renderAddressFormComponent({ formFields: [...formFields, phoneFormFieldMock] });
Expand All @@ -189,15 +194,15 @@ describe('AddressForm Component', () => {
expect(screen.getByTestId('phoneInput-text')).toBeInTheDocument();
});

it('renders new phone number field when custom checkout provider is not PayPal Fastlane', () => {
it('renders legacy phone field when isPhoneNumberValidationEnabled is missing from config', () => {
jest.spyOn(checkoutService.getState().data, 'getConfig').mockReturnValue(
getConfigMockWithPhoneExperimentTrue('100%_definitely_not_fastlane'),
getStoreConfig(),
);

renderAddressFormComponent({ formFields: [...formFields, phoneFormFieldMock] });

expect(screen.getByTestId('intl-tel-input-mock')).toBeInTheDocument();
expect(screen.queryByTestId('phoneInput-text')).not.toBeInTheDocument();
expect(screen.queryByTestId('intl-tel-input-mock')).not.toBeInTheDocument();
expect(screen.getByTestId('phoneInput-text')).toBeInTheDocument();
});
});

Expand Down
19 changes: 3 additions & 16 deletions packages/core/src/app/address/AddressForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
useThemeContext,
} from '@bigcommerce/checkout/contexts';
import { TranslatedString } from '@bigcommerce/checkout/locale';
import { isPayPalFastlaneMethod } from '@bigcommerce/checkout/paypal-fastlane-integration';
import {
type AutocompleteItem,
CheckboxFormField,
Expand All @@ -20,7 +19,6 @@ import {
import { isExperimentEnabled } from '@bigcommerce/checkout/utility';

import { EMPTY_ARRAY, isFloatingLabelEnabled } from '../common/utility';
import getProviderWithCustomCheckout from '../payment/getProviderWithCustomCheckout';

import {
type AddressFormProps,
Expand Down Expand Up @@ -67,17 +65,8 @@ const AddressForm: React.FC<AddressFormProps> = ({
const isFloatingLabelEnabledValue = config
? isFloatingLabelEnabled(config.checkoutSettings)
: false;
const isPayPalFastlaneEnabled = isPayPalFastlaneMethod(
getProviderWithCustomCheckout(config?.checkoutSettings.providerWithCustomCheckout),
);
// PayPal Fastlane stores keep the legacy phone input for now, due to incident
const isNewPhoneValidationExperimentEnabled =
!isPayPalFastlaneEnabled &&
isExperimentEnabled(
config?.checkoutSettings,
'CHECKOUT-9019.use_new_phone_number_validation',
false,
);
const isPhoneNumberValidationEnabled =
config?.checkoutSettings.isPhoneNumberValidationEnabled ?? false;
const isNewGooglePlacesApiEnabled = isExperimentEnabled(
config?.checkoutSettings,
'CHECKOUT-10026.new_google_places_api',
Expand Down Expand Up @@ -236,9 +225,7 @@ const AddressForm: React.FC<AddressFormProps> = ({
inputId={getAddressFormFieldInputId(addressFieldName)}
// stateOrProvince can sometimes be a dropdown or input, so relying on id is not sufficient
isFloatingLabelEnabled={isFloatingLabelEnabledValue}
isNewPhoneValidationExperimentEnabled={
isNewPhoneValidationExperimentEnabled
}
isPhoneNumberValidationEnabled={isPhoneNumberValidationEnabled}
key={`${field.id}-${field.name}`}
label={
field.custom || isExtraField(field) ? (
Expand Down
49 changes: 23 additions & 26 deletions packages/ui/src/form/DynamicFormField/DynamicFormField.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ describe('DynamicFormField Component', () => {
).toBeInTheDocument();
});

describe('phone validation with isNewPhoneValidationExperimentEnabled', () => {
describe('phone validation with isPhoneNumberValidationEnabled', () => {
// Phone field: fieldType 'text' + name containing 'phone' resolves to TELEPHONE internally
const phoneFieldMock: FormFieldType = {
custom: false,
Expand Down Expand Up @@ -187,7 +187,7 @@ describe('DynamicFormField Component', () => {

const renderMockFormField = (overrides: {
field: FormFieldType;
isNewPhoneValidationExperimentEnabled: boolean;
isPhoneNumberValidationEnabled: boolean;
placeholder?: string;
}) =>
render(
Expand All @@ -197,8 +197,8 @@ describe('DynamicFormField Component', () => {
<FormProvider initialIsSubmitted>
<DynamicFormField
field={overrides.field}
isNewPhoneValidationExperimentEnabled={
overrides.isNewPhoneValidationExperimentEnabled
isPhoneNumberValidationEnabled={
overrides.isPhoneNumberValidationEnabled
}
placeholder={overrides.placeholder}
/>
Expand All @@ -217,13 +217,13 @@ describe('DynamicFormField Component', () => {
mockSetCountry.mockClear();
});

it('shows a validation error when the phone number is invalid and the experiment is enabled', async () => {
it('shows a validation error when the phone number is invalid and the setting is enabled', async () => {
mockGetSelectedCountryData.mockReturnValue({ iso2: 'us' });
mockIsValidNumber.mockReturnValue(false);

renderMockFormField({
field: { ...phoneFieldMock, required: true },
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
});

fireEvent.change(screen.getByTestId('phone-text'), {
Expand All @@ -242,7 +242,7 @@ describe('DynamicFormField Component', () => {

renderMockFormField({
field: { ...phoneFieldMock, required: true },
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
});

fireEvent.change(screen.getByTestId('phone-text'), {
Expand All @@ -261,7 +261,7 @@ describe('DynamicFormField Component', () => {

renderMockFormField({
field: phoneFieldMock,
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
});

// Leave value empty, submit directly
Expand All @@ -272,15 +272,15 @@ describe('DynamicFormField Component', () => {
});
});

it('does not validate phone when the experiment flag is disabled', async () => {
it('does not validate phone when the setting is disabled', async () => {
mockIsValidNumber.mockReturnValue(false);

renderMockFormField({
field: phoneFieldMock,
isNewPhoneValidationExperimentEnabled: false,
isPhoneNumberValidationEnabled: false,
});

// Experiment off: IntlTelInput is not rendered, regular tel input is used instead
// Setting off: IntlTelInput is not rendered, regular tel input is used instead
fireEvent.change(screen.getByRole('textbox'), { target: { value: '123' } });
await userEvent.click(screen.getByText('Submit'));

Expand All @@ -289,61 +289,58 @@ describe('DynamicFormField Component', () => {
});
});

it('renders the legacy phone input when the field has a max length, even when the experiment is enabled', async () => {
it('renders the validated phone input when the field has a max length', async () => {
mockGetSelectedCountryData.mockReturnValue({ iso2: 'us' });
mockIsValidNumber.mockReturnValue(false);

const { container } = renderMockFormField({
field: { ...phoneFieldMock, maxLength: 10 },
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
});

const input = screen.getByTestId('phone-text');

expect(input).toHaveAttribute('type', 'tel');
expect(input).toHaveAttribute('maxlength', '10');
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
expect(container.querySelector('.iti-wrapper')).not.toBeInTheDocument();
expect(container.querySelector('.iti-wrapper')).toBeInTheDocument();

// The legacy input does not apply IntlTelInput phone validation
fireEvent.change(input, { target: { value: '123' } });
await userEvent.click(screen.getByText('Submit'));

await waitFor(() => {
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
expect(screen.getByRole('alert')).toBeInTheDocument();
});
});

it('renders the legacy phone input when the field has a placeholder, even when the experiment is enabled', async () => {
it('renders the validated phone input when the field has a placeholder', async () => {
mockGetSelectedCountryData.mockReturnValue({ iso2: 'us' });
mockIsValidNumber.mockReturnValue(false);

const { container } = renderMockFormField({
field: { ...phoneFieldMock, default: '1231232' },
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
placeholder: '1231232',
});

const input = screen.getByTestId('phone-text');

expect(input).toHaveAttribute('type', 'tel');
expect(input).toHaveAttribute('placeholder', '1231232');
// eslint-disable-next-line testing-library/no-container, testing-library/no-node-access
expect(container.querySelector('.iti-wrapper')).not.toBeInTheDocument();
expect(container.querySelector('.iti-wrapper')).toBeInTheDocument();

// The legacy input does not apply IntlTelInput phone validation
fireEvent.change(input, { target: { value: '123' } });
await userEvent.click(screen.getByText('Submit'));

await waitFor(() => {
expect(screen.queryByRole('alert')).not.toBeInTheDocument();
expect(screen.getByRole('alert')).toBeInTheDocument();
});
});

it('does not validate phone for non-telephone fields even when experiment is enabled', async () => {
it('does not validate phone for non-telephone fields even when the setting is enabled', async () => {
mockIsValidNumber.mockReturnValue(false);

renderMockFormField({
field: nonPhoneFieldMock,
isNewPhoneValidationExperimentEnabled: true,
isPhoneNumberValidationEnabled: true,
});

fireEvent.change(screen.getByRole('textbox'), { target: { value: 'John' } });
Expand Down
6 changes: 3 additions & 3 deletions packages/ui/src/form/DynamicFormField/DynamicFormField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface DynamicFormFieldProps {
placeholder?: string;
label?: ReactNode;
isFloatingLabelEnabled?: boolean;
isNewPhoneValidationExperimentEnabled?: boolean;
isPhoneNumberValidationEnabled?: boolean;
selectedCountry?: string;
onChange?(value: string | string[]): void;
}
Expand All @@ -51,7 +51,7 @@ const DynamicFormField: FunctionComponent<DynamicFormFieldProps> = ({
extraClass,
isFloatingLabelEnabled,
selectedCountry,
isNewPhoneValidationExperimentEnabled = false,
isPhoneNumberValidationEnabled = false,
}) => {
const fieldInputId = inputId || name;
const fieldName = parentFieldName ? `${parentFieldName}.${name}` : name;
Expand Down Expand Up @@ -117,7 +117,7 @@ const DynamicFormField: FunctionComponent<DynamicFormFieldProps> = ({
fieldType={fieldType}
id={fieldInputId}
isFloatingLabelEnabled={isFloatingLabelSupportedFieldType}
isNewPhoneValidationExperimentEnabled={isNewPhoneValidationExperimentEnabled}
isPhoneNumberValidationEnabled={isPhoneNumberValidationEnabled}
label={labelComponent}
max={max}
maxLength={maxLength}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ interface DynamicFormFieldSelectorProps {
min?: FormFieldType['min'];
maxLength?: FormFieldType['maxLength'];
isFloatingLabelEnabled?: boolean;
isNewPhoneValidationExperimentEnabled: boolean;
isPhoneNumberValidationEnabled: boolean;
selectedCountry?: string;
onChange?(value: string | string[]): void;
}
Expand All @@ -41,16 +41,12 @@ export const DynamicFormFieldSelector: FunctionComponent<DynamicFormFieldSelecto
min,
maxLength,
isFloatingLabelEnabled,
isNewPhoneValidationExperimentEnabled,
isPhoneNumberValidationEnabled,
selectedCountry,
onChange,
}) => {
// skipped for stores with maxLength as it caused formatting issues
// and for stores with a default value placeholder (to de-risk)
const isNewPhoneFieldWithValidation =
isNewPhoneValidationExperimentEnabled &&
!maxLength &&
!placeholder &&
isPhoneNumberValidationEnabled &&
dynamicFormFieldType === DynamicFormFieldType.TELEPHONE;

const renderInput = useCallback(
Expand Down