Skip to content

Commit

Permalink
fix: make textfield a controlled component (#830)
Browse files Browse the repository at this point in the history
* fix: make textfield a controlled component

* fix: update cypress form test

* fix: cypress form test

* fix: update timestamp assertion

* chore: reset test data

Co-authored-by: Scott Young <scoyou@amazon.com>
  • Loading branch information
scottyoung and Scott Young authored Dec 13, 2022
1 parent 6898d10 commit 1dcda83
Show file tree
Hide file tree
Showing 10 changed files with 419 additions and 179 deletions.

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
/*
Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
Licensed under the Apache License, Version 2.0 (the "License").
You may not use this file except in compliance with the License.
You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/
import { renderValueAttribute, renderDefaultValueAttribute } from '../../forms/component-helper';

describe('render value attribute', () => {
it('should render TextField with value attribute', () => {
const attr = renderValueAttribute({
fieldConfig: {
validationRules: [],
componentType: 'TextField',
},
componentName: 'animalName',
});

expect(attr).toHaveProperty('initializer.expression.escapedText', 'animalName');
});
});

describe('render default value attribute', () => {
it('should render TextField with the value attribute', () => {
const attr = renderDefaultValueAttribute(
'buildingName',
{ validationRules: [], componentType: 'TextField' },
'TextField',
);

expect(attr).toHaveProperty('initializer.expression.escapedText', 'buildingName');
});
});
14 changes: 13 additions & 1 deletion packages/codegen-ui-react/lib/__tests__/forms/form-state.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
limitations under the License.
*/
import { factory } from 'typescript';
import { buildNestedStateSet, setFieldState } from '../../forms/form-state';
import { buildNestedStateSet, setFieldState, getDefaultValueExpression } from '../../forms/form-state';
import { genericPrinter } from '../__utils__';

describe('nested state', () => {
Expand Down Expand Up @@ -57,3 +57,15 @@ describe('set field state', () => {
expect(response).toMatchSnapshot();
});
});

describe('get default values', () => {
it('should generate the proper default value for a TextField', () => {
const expression = getDefaultValueExpression('name', 'TextField');
expect(expression).toMatchObject({ text: '' });
});

it('should generate the proper default value for a SliderField', () => {
const expression = getDefaultValueExpression('name', 'SliderField');
expect(expression).toMatchObject({ text: '0' });
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ describe('amplify form renderer tests', () => {
'datastore/post',
);
expect(componentText).toContain('DataStore.save');
expect(componentText).toContain('resetStateValues();');
expect(componentText).toMatchSnapshot();
expect(declaration).toMatchSnapshot();
});
Expand Down Expand Up @@ -57,22 +58,24 @@ describe('amplify form renderer tests', () => {
expect(declaration).toMatchSnapshot();
});

it('should render a form with multiple date types', () => {
it('should render a form with multiple date types on create form', () => {
const { componentText, declaration } = generateWithAmplifyFormRenderer(
'forms/input-gallery-create',
'datastore/input-gallery',
);
expect(componentText).toContain('DataStore.save');
expect(componentText).toContain('const convertToLocal');
expect(componentText).toMatchSnapshot();
expect(declaration).toMatchSnapshot();
});

it('should render a form with multiple date types', () => {
it('should render a form with multiple date types on update form', () => {
const { componentText, declaration } = generateWithAmplifyFormRenderer(
'forms/input-gallery-update',
'datastore/input-gallery',
);
expect(componentText).toContain('DataStore.save');
expect(componentText).toContain('const convertToLocal');
expect(componentText).toMatchSnapshot();
expect(declaration).toMatchSnapshot();
});
Expand Down Expand Up @@ -132,13 +135,13 @@ describe('amplify form renderer tests', () => {
expect(declaration).toMatchSnapshot();
});

it('should render nested json fields', () => {
it('should render nested json fields for create form', () => {
const { componentText, declaration } = generateWithAmplifyFormRenderer('forms/bio-nested-create', undefined);
expect(componentText).toMatchSnapshot();
expect(declaration).toMatchSnapshot();
});

it('should render nested json fields', () => {
it('should render nested json fields for update form', () => {
const { componentText, declaration } = generateWithAmplifyFormRenderer('forms/bio-nested-update', undefined);
expect(componentText).toMatchSnapshot();
expect(declaration).toMatchSnapshot();
Expand Down
58 changes: 51 additions & 7 deletions packages/codegen-ui-react/lib/forms/component-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,18 @@
*/

import { FieldConfigMetadata, StudioDataSourceType, StudioFormActionType } from '@aws-amplify/codegen-ui';
import { BinaryExpression, factory, Identifier, JsxAttribute, SyntaxKind } from 'typescript';
import { resetValuesName } from './form-state';
import { BinaryExpression, factory, Identifier, JsxAttribute, SyntaxKind, ElementAccessExpression } from 'typescript';
import { getCurrentValueName, resetValuesName } from './form-state';
import { FIELD_TYPE_TO_TYPESCRIPT_MAP } from './typescript-type-map';

export const ControlledComponents = ['StepperField', 'SliderField', 'SelectField', 'ToggleButton', 'SwitchField'];
export const ControlledComponents = [
'StepperField',
'SliderField',
'SelectField',
'ToggleButton',
'SwitchField',
'TextField',
];

/**
* given the component returns true if the component is a controlled component
Expand Down Expand Up @@ -54,15 +61,22 @@ export const convertedValueAttributeMap: Record<string, (valueIdentifier: Identi
* @param { dataType } the dataType
* @returns
*/
export const renderDefaultValueAttribute = (stateName: string, { dataType }: FieldConfigMetadata) => {
export const renderDefaultValueAttribute = (
stateName: string,
{ dataType }: FieldConfigMetadata,
componentType: string,
) => {
const identifier = factory.createIdentifier(stateName);
let expression = factory.createJsxExpression(undefined, identifier);

if (dataType && typeof dataType !== 'object' && convertedValueAttributeMap[dataType]) {
expression = factory.createJsxExpression(undefined, convertedValueAttributeMap[dataType](identifier));
}

return factory.createJsxAttribute(factory.createIdentifier('defaultValue'), expression);
return factory.createJsxAttribute(
componentType === 'TextField' ? factory.createIdentifier('value') : factory.createIdentifier('defaultValue'),
expression,
);
};

export const renderValueAttribute = ({
Expand All @@ -75,10 +89,33 @@ export const renderValueAttribute = ({
currentValueIdentifier?: Identifier;
}): JsxAttribute | undefined => {
const componentType = fieldConfig.studioFormComponentType ?? fieldConfig.componentType;
const { dataType } = fieldConfig;
const shouldGetForUncontrolled = fieldConfig.isArray;

const valueIdentifier = currentValueIdentifier || getValueIdentifier(componentName, componentType);

let renderedFieldName = fieldConfig.sanitizedFieldName || componentName;
if (fieldConfig.isArray) {
renderedFieldName = getCurrentValueName(renderedFieldName);
}
let fieldNameIdentifier: Identifier | ElementAccessExpression = factory.createIdentifier(renderedFieldName);
if (componentName.includes('.')) {
const [parent, child] = componentName.split('.');
fieldNameIdentifier = factory.createElementAccessExpression(
factory.createIdentifier(parent),
factory.createStringLiteral(child),
);
}

let controlledExpression = factory.createJsxExpression(undefined, fieldNameIdentifier);

if (dataType && typeof dataType !== 'object' && convertedValueAttributeMap[dataType]) {
controlledExpression = factory.createJsxExpression(
undefined,
convertedValueAttributeMap[dataType](valueIdentifier),
);
}

const controlledComponentToAttributesMap: { [key: string]: JsxAttribute } = {
ToggleButton: factory.createJsxAttribute(
factory.createIdentifier('isPressed'),
Expand All @@ -104,6 +141,15 @@ export const renderValueAttribute = ({
factory.createIdentifier('checked'),
factory.createJsxExpression(undefined, valueIdentifier),
),
TextField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
DateTimeField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
IPAddressField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
DateField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
TimeField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
NumberField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
URLField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
PhoneNumberField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
EmailField: factory.createJsxAttribute(factory.createIdentifier('value'), controlledExpression),
};

if (controlledComponentToAttributesMap[componentType]) {
Expand All @@ -112,8 +158,6 @@ export const renderValueAttribute = ({

// TODO: all components should be controlled once conversions are solid
if (shouldGetForUncontrolled) {
const { dataType } = fieldConfig;

let expression = factory.createJsxExpression(undefined, valueIdentifier);

if (dataType && typeof dataType !== 'object' && convertedValueAttributeMap[dataType]) {
Expand Down
45 changes: 43 additions & 2 deletions packages/codegen-ui-react/lib/forms/form-renderer-helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export const addFormAttributes = (component: StudioComponent | StudioComponentCh
}

if (formMetadata.formActionType === 'update' && !fieldConfig.isArray && !isControlledComponent(componentType)) {
attributes.push(renderDefaultValueAttribute(renderedVariableName, fieldConfig));
attributes.push(renderDefaultValueAttribute(renderedVariableName, fieldConfig, componentType));
}
attributes.push(buildOnChangeStatement(component, formMetadata.fieldConfigs));
attributes.push(buildOnBlurStatement(componentName, fieldConfig));
Expand Down Expand Up @@ -238,11 +238,52 @@ export const addFormAttributes = (component: StudioComponent | StudioComponentCh
attributes.push(flexGapAttribute);
}
}
/*
onClick={(event) => {
event.preventDefault();
resetStateValues();
}}
*/
if (componentName === 'ClearButton' || componentName === 'ResetButton') {
attributes.push(
factory.createJsxAttribute(
factory.createIdentifier('onClick'),
factory.createJsxExpression(undefined, resetValuesName),
factory.createJsxExpression(
undefined,
factory.createArrowFunction(
undefined,
undefined,
[
factory.createParameterDeclaration(
undefined,
undefined,
undefined,
factory.createIdentifier('event'),
undefined,
factory.createTypeReferenceNode(factory.createIdentifier('SyntheticEvent'), undefined),
undefined,
),
],
undefined,
factory.createToken(SyntaxKind.EqualsGreaterThanToken),
factory.createBlock(
[
factory.createExpressionStatement(
factory.createCallExpression(
factory.createPropertyAccessExpression(
factory.createIdentifier('event'),
factory.createIdentifier('preventDefault'),
),
undefined,
[],
),
),
factory.createExpressionStatement(factory.createCallExpression(resetValuesName, undefined, [])),
],
true,
),
),
),
),
);
if (formMetadata.formActionType === 'update' && formMetadata.dataType.dataSourceType === 'DataStore') {
Expand Down
1 change: 1 addition & 0 deletions packages/codegen-ui-react/lib/forms/form-state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ export const getDefaultValueExpression = (
StepperField: factory.createNumericLiteral(0),
SliderField: factory.createNumericLiteral(0),
CheckboxField: factory.createFalse(),
TextField: factory.createStringLiteral(''),
};

if (isArray) {
Expand Down
16 changes: 2 additions & 14 deletions packages/codegen-ui-react/lib/forms/react-form-renderer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -528,23 +528,11 @@ export abstract class ReactFormTemplateRenderer extends StudioTemplateRenderer<
// timestamp type takes precedence over datetime as it includes formatter for datetime
// we include both the timestamp conversion and local date formatter
if (dataTypesMap.AWSTimestamp) {
// helper needed if update form
// or, if create form and the field is an array and therefore controlled
if (
formActionType === 'update' ||
dataTypesMap.AWSTimestamp.some((fieldName) => formMetadata.fieldConfigs[fieldName].isArray)
) {
statements.push(convertTimeStampToDateAST, convertToLocalAST);
}
statements.push(convertTimeStampToDateAST, convertToLocalAST);
}
// if we only have date time then we only need the local conversion
else if (dataTypesMap.AWSDateTime) {
if (
formActionType === 'update' ||
dataTypesMap.AWSDateTime.some((fieldName) => formMetadata.fieldConfigs[fieldName].isArray)
) {
statements.push(convertToLocalAST);
}
statements.push(convertToLocalAST);
}

return statements;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ describe('getFormDefinitionInputElement', () => {
isRequired: true,
isReadOnly: false,
placeholder: 'MyPlaceholder',
defaultValue: 'MyDefaultValue',
value: 'MyDefaultValue',
},
};

Expand All @@ -114,7 +114,6 @@ describe('getFormDefinitionInputElement', () => {
label: 'MyLabel',
descriptiveText: 'MyDescriptiveText',
placeholder: 'MyPlaceholder',
defaultValue: 'MyDefaultValue',
},
studioFormComponentType: 'TextField',
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ describe('Forms', () => {
getInputByLabel('Aws date').type('2022-10-12');
getInputByLabel('Aws time').type('10:12');
getInputByLabel('Aws date time').type('2017-06-01T08:30');
getInputByLabel('Aws timestamp').type('2022-12-01T10:30');
getInputByLabel('Aws email').type('myemail@yahoo.com');
getInputByLabel('Aws url').type('https://amazon.com');
getInputByLabel('Aws ip address').type('192.0.2.146');
Expand All @@ -120,6 +121,7 @@ describe('Forms', () => {
expect(record.awsDate).to.equal('2022-10-12');
expect(record.awsTime).to.equal('10:12');
expect(record.awsDateTime).to.equal('2017-06-01T08:30:00.000Z');
expect(record.awsTimestamp).to.equal(1669890600000);
expect(record.awsEmail).to.equal('myemail@yahoo.com');
expect(record.awsUrl).to.equal('https://amazon.com');
expect(record.awsIPAddress).to.equal('192.0.2.146');
Expand Down

0 comments on commit 1dcda83

Please sign in to comment.