Skip to content

Commit a38e111

Browse files
mmalerbammalerba
committed
fix(material/schematics): handle form-field appearance
1 parent 6642a76 commit a38e111

File tree

8 files changed

+219
-36
lines changed

8 files changed

+219
-36
lines changed

‎integration/mdc-migration/golden/src/app/components/form-field/form-field.component.html

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@ <h2>Form field example</h2>
22
<mat-form-field hintLabel="Max 10 characters" appearance="fill">
33
<mat-label>Enter some input</mat-label>
44
<input matInput #input maxlength="10" placeholder="Ex. Nougat">
5-
<mat-hint align="end">{{input.value?.length || 0}}/10</mat-hint>
5+
<mat-hint align="end">{{input.value.length}}/10</mat-hint>
66
</mat-form-field>
7+
<mat-form-field appearance="outline"><input matInput></mat-form-field>
8+
<mat-form-field><input matInput></mat-form-field>
9+
<mat-form-field><input matInput></mat-form-field>

‎integration/mdc-migration/sample-project/src/app/components/form-field/form-field.component.html

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,8 @@ <h2>Form field example</h2>
22
<mat-form-field hintLabel="Max 10 characters" appearance="fill">
33
<mat-label>Enter some input</mat-label>
44
<input matInput #input maxlength="10" placeholder="Ex. Nougat">
5-
<mat-hint align="end">{{input.value?.length || 0}}/10</mat-hint>
5+
<mat-hint align="end">{{input.value.length}}/10</mat-hint>
66
</mat-form-field>
7+
<mat-form-field appearance="outline"><input matInput></mat-form-field>
8+
<mat-form-field appearance="standard"><input matInput></mat-form-field>
9+
<mat-form-field appearance="legacy"><input matInput></mat-form-field>

‎src/material/schematics/ng-generate/mdc-migration/rules/components/card/card-template.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
import * as compiler from '@angular/compiler';
1010
import {TemplateMigrator} from '../../template-migrator';
11-
import {addAttribute, visitElements} from '../../tree-traversal';
11+
import {updateAttribute, visitElements} from '../../tree-traversal';
1212
import {Update} from '../../../../../migration-utilities';
1313

1414
export class CardTemplateMigrator extends TemplateMigrator {
@@ -22,7 +22,7 @@ export class CardTemplateMigrator extends TemplateMigrator {
2222

2323
updates.push({
2424
offset: node.startSourceSpan.start.offset,
25-
updateFn: html => addAttribute(html, node, 'appearance', 'outlined'),
25+
updateFn: html => updateAttribute(html, node, 'appearance', () => 'outlined'),
2626
});
2727
});
2828

‎src/material/schematics/ng-generate/mdc-migration/rules/components/form-field/form-field-template.spec.ts

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import {createTestApp, patchDevkitTreeToExposeTypeScript} from '@angular/cdk/schematics/testing';
2+
import {SchematicTestRunner, UnitTestTree} from '@angular-devkit/schematics/testing';
3+
import {
4+
APP_MODULE_FILE,
5+
createNewTestRunner,
6+
migrateComponents,
7+
TEMPLATE_FILE,
8+
} from '../test-setup-helper';
9+
10+
describe('form-field template migrator', () => {
11+
let runner: SchematicTestRunner;
12+
let cliAppTree: UnitTestTree;
13+
14+
async function runMigrationTest(oldFileContent: string, newFileContent: string) {
15+
cliAppTree.overwrite(TEMPLATE_FILE, oldFileContent);
16+
const tree = await migrateComponents(['form-field'], runner, cliAppTree);
17+
expect(tree.readContent(TEMPLATE_FILE)).toBe(newFileContent);
18+
}
19+
20+
beforeEach(async () => {
21+
runner = createNewTestRunner();
22+
cliAppTree = patchDevkitTreeToExposeTypeScript(await createTestApp(runner));
23+
});
24+
25+
it('should not update other elements', async () => {
26+
await runMigrationTest(
27+
'<mat-card appearance="raised"></mat-card>',
28+
'<mat-card appearance="raised"></mat-card>',
29+
);
30+
});
31+
32+
it('should not update default appearance', async () => {
33+
await runMigrationTest(
34+
'<mat-form-field appearance="outline"></mat-form-field>',
35+
'<mat-form-field appearance="outline"></mat-form-field>',
36+
);
37+
});
38+
39+
it('should not update outline appearance', async () => {
40+
await runMigrationTest(
41+
'<mat-form-field appearance="outline"></mat-form-field>',
42+
'<mat-form-field appearance="outline"></mat-form-field>',
43+
);
44+
});
45+
46+
it('should not update fill appearance', async () => {
47+
await runMigrationTest(
48+
'<mat-form-field appearance="fill"></mat-form-field>',
49+
'<mat-form-field appearance="fill"></mat-form-field>',
50+
);
51+
});
52+
53+
it('should update standard appearance', async () => {
54+
await runMigrationTest(
55+
'<mat-form-field appearance="standard"></mat-form-field>',
56+
'<mat-form-field></mat-form-field>',
57+
);
58+
});
59+
60+
it('should update legacy appearance', async () => {
61+
await runMigrationTest(
62+
'<mat-form-field appearance="legacy"></mat-form-field>',
63+
'<mat-form-field></mat-form-field>',
64+
);
65+
});
66+
});

‎src/material/schematics/ng-generate/mdc-migration/rules/components/form-field/form-field-template.ts

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* @license
3+
* Copyright Google LLC All Rights Reserved.
4+
*
5+
* Use of this source code is governed by an MIT-style license that can be
6+
* found in the LICENSE file at https://angular.io/license
7+
*/
8+
9+
import * as compiler from '@angular/compiler';
10+
import {TemplateMigrator} from '../../template-migrator';
11+
import {updateAttribute, visitElements} from '../../tree-traversal';
12+
import {Update} from '../../../../../migration-utilities';
13+
14+
export class FormFieldTemplateMigrator extends TemplateMigrator {
15+
getUpdates(ast: compiler.ParsedTemplate): Update[] {
16+
const updates: Update[] = [];
17+
18+
visitElements(ast.nodes, (node: compiler.TmplAstElement) => {
19+
if (node.name !== 'mat-form-field') {
20+
return;
21+
}
22+
23+
updates.push({
24+
offset: node.startSourceSpan.start.offset,
25+
updateFn: html =>
26+
updateAttribute(html, node, 'appearance', old =>
27+
['legacy', 'standard'].includes(old || '') ? null : old,
28+
),
29+
});
30+
});
31+
32+
return updates;
33+
}
34+
}

‎src/material/schematics/ng-generate/mdc-migration/rules/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import {TabsStylesMigrator} from './components/tabs/tabs-styles';
3434
import {TooltipStylesMigrator} from './components/tooltip/tooltip-styles';
3535
import {OptgroupStylesMigrator} from './components/optgroup/optgroup-styles';
3636
import {OptionStylesMigrator} from './components/option/option-styles';
37+
import {FormFieldTemplateMigrator} from './components/form-field/form-field-template';
3738

3839
/** Contains the migrators to migrate a single component. */
3940
export interface ComponentMigrator {
@@ -121,6 +122,7 @@ export const MIGRATORS: ComponentMigrator[] = [
121122
{
122123
component: 'form-field',
123124
styles: new FormFieldStylesMigrator(),
125+
template: new FormFieldTemplateMigrator(),
124126
},
125127
{
126128
component: 'input',

‎src/material/schematics/ng-generate/mdc-migration/rules/tree-traversal.spec.ts

Lines changed: 73 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import {
2-
addAttribute,
2+
updateAttribute,
33
visitElements,
44
parseTemplate,
55
replaceStartTag,
@@ -21,7 +21,21 @@ function runTagNameDuplicationTest(html: string, result: string): void {
2121

2222
function runAddAttributeTest(html: string, result: string): void {
2323
visitElements(parseTemplate(html).nodes, undefined, node => {
24-
html = addAttribute(html, node, 'attr', 'val');
24+
html = updateAttribute(html, node, 'add', () => 'val');
25+
});
26+
expect(html).toBe(result);
27+
}
28+
29+
function runRemoveAttributeTest(html: string, result: string): void {
30+
visitElements(parseTemplate(html).nodes, undefined, node => {
31+
html = updateAttribute(html, node, 'rm', () => null);
32+
});
33+
expect(html).toBe(result);
34+
}
35+
36+
function runChangeAttributeTest(html: string, result: string): void {
37+
visitElements(parseTemplate(html).nodes, undefined, node => {
38+
html = updateAttribute(html, node, 'change', old => (old == ':(' ? ':)' : old));
2539
});
2640
expect(html).toBe(result);
2741
}
@@ -92,39 +106,85 @@ describe('#visitElements', () => {
92106

93107
describe('add attribute tests', () => {
94108
it('should handle single element', async () => {
95-
runAddAttributeTest('<a></a>', '<a attr="val"></a>');
109+
runAddAttributeTest('<a></a>', '<a add="val"></a>');
96110
});
97111

98112
it('should handle multiple unnested', async () => {
99-
runAddAttributeTest('<a></a><b></b>', '<a attr="val"></a><b attr="val"></b>');
113+
runAddAttributeTest('<a></a><b></b>', '<a add="val"></a><b add="val"></b>');
100114
});
101115

102116
it('should handle multiple nested', async () => {
103-
runAddAttributeTest('<a><b></b></a>', '<a attr="val"><b attr="val"></b></a>');
117+
runAddAttributeTest('<a><b></b></a>', '<a add="val"><b add="val"></b></a>');
104118
});
105119

106120
it('should handle multiple nested and unnested', async () => {
107121
runAddAttributeTest(
108122
'<a><b></b><c></c></a>',
109-
'<a attr="val"><b attr="val"></b><c attr="val"></c></a>',
123+
'<a add="val"><b add="val"></b><c add="val"></c></a>',
110124
);
111125
});
112126

113127
it('should handle adding multiple attrs to a single element', async () => {
114128
let html = '<a></a>';
115129
visitElements(parseTemplate(html).nodes, undefined, node => {
116-
html = addAttribute(html, node, 'attr1', 'val1');
117-
html = addAttribute(html, node, 'attr2', 'val2');
130+
html = updateAttribute(html, node, 'attr1', () => 'val1');
131+
html = updateAttribute(html, node, 'attr2', () => 'val2');
118132
});
119133
expect(html).toBe('<a attr2="val2" attr1="val1"></a>');
120134
});
121135

122136
it('should replace value of existing attribute', async () => {
123-
runAddAttributeTest('<a attr="default"></a>', '<a attr="val"></a>');
137+
runAddAttributeTest('<a add="default"></a>', '<a add="val"></a>');
124138
});
125139

126140
it('should add value to existing attribute that does not have a value', async () => {
127-
runAddAttributeTest('<a attr></a>', '<a attr="val"></a>');
141+
runAddAttributeTest('<a add></a>', '<a add="val"></a>');
142+
});
143+
});
144+
145+
describe('remove attribute tests', () => {
146+
it('should remove attribute', () => {
147+
runRemoveAttributeTest('<a rm="something"></a>', '<a></a>');
148+
});
149+
150+
it('should remove empty attribute', () => {
151+
runRemoveAttributeTest('<a rm></a>', '<a></a>');
152+
});
153+
154+
it('should remove unquoted attribute', () => {
155+
runRemoveAttributeTest('<a rm=3></a>', '<a></a>');
156+
});
157+
158+
it('should remove value-less attribute', () => {
159+
runRemoveAttributeTest('<a rm></a>', '<a></a>');
160+
});
161+
162+
it('should not remove other attributes', () => {
163+
runRemoveAttributeTest(
164+
`
165+
<a
166+
first="1"
167+
rm="2"
168+
last="3">
169+
</a>
170+
`,
171+
`
172+
<a
173+
first="1"
174+
last="3">
175+
</a>
176+
`,
177+
);
178+
});
179+
});
180+
181+
describe('change attribute tests', () => {
182+
it('should change attribute with matching value', () => {
183+
runChangeAttributeTest('<a change=":("></a>', '<a change=":)"></a>');
184+
});
185+
186+
it('should not change attribute with non-matching value', () => {
187+
runChangeAttributeTest('<a change="x"></a>', '<a change="x"></a>');
128188
});
129189
});
130190

@@ -139,7 +199,7 @@ describe('#visitElements', () => {
139199
`,
140200
`
141201
<a
142-
attr="val"
202+
add="val"
143203
class="a"
144204
aria-label="a"
145205
aria-describedby="a"
@@ -157,8 +217,8 @@ describe('#visitElements', () => {
157217
></a>
158218
`;
159219
visitElements(parseTemplate(html).nodes, undefined, node => {
160-
html = addAttribute(html, node, 'attr1', 'val1');
161-
html = addAttribute(html, node, 'attr2', 'val2');
220+
html = updateAttribute(html, node, 'attr1', () => 'val1');
221+
html = updateAttribute(html, node, 'attr2', () => 'val2');
162222
});
163223
expect(html).toBe(`
164224
<a

‎src/material/schematics/ng-generate/mdc-migration/rules/tree-traversal.ts

Lines changed: 34 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -91,50 +91,65 @@ export function replaceEndTag(html: string, node: TmplAstElement, tag: string):
9191
* @param html The template html to be updated.
9292
* @param node The node to be updated.
9393
* @param name The name of the attribute.
94-
* @param value The value of the attribute.
94+
* @param update The function that determines how to update the value.
9595
* @returns The updated template html.
9696
*/
97-
export function addAttribute(
97+
export function updateAttribute(
9898
html: string,
9999
node: TmplAstElement,
100100
name: string,
101-
value: string,
101+
update: (old: string | null) => string | null,
102102
): string {
103103
const existingAttr = node.attributes.find(currentAttr => currentAttr.name === name);
104104

105-
if (existingAttr) {
105+
if (existingAttr && existingAttr.keySpan) {
106106
// If the attribute has a value already, replace it.
107107
if (existingAttr.valueSpan) {
108-
return (
109-
html.slice(0, existingAttr.valueSpan.start.offset) +
110-
value +
111-
html.slice(existingAttr.valueSpan.end.offset)
112-
);
113-
} else if (existingAttr.keySpan) {
114-
// Otherwise add a value to a value-less attribute. Note that the `keySpan` null check is
115-
// only necessary for the compiler. Technically an attribute should always have a key.
116-
return (
117-
html.slice(0, existingAttr.keySpan.end.offset) +
118-
`="${value}"` +
119-
html.slice(existingAttr.keySpan.end.offset)
120-
);
108+
const updatedValue = update(existingAttr.valueSpan.toString());
109+
if (updatedValue != null) {
110+
return (
111+
html.slice(0, existingAttr.valueSpan.start.offset) +
112+
updatedValue +
113+
html.slice(existingAttr.valueSpan.end.offset)
114+
);
115+
} else {
116+
return (
117+
html.slice(0, existingAttr.sourceSpan.start.offset).trimEnd() +
118+
html.slice(existingAttr.sourceSpan.end.offset)
119+
);
120+
}
121+
} else {
122+
const updatedValue = update('');
123+
if (updatedValue != null) {
124+
return (
125+
html.slice(0, existingAttr.keySpan.end.offset) +
126+
`="${updatedValue}"` +
127+
html.slice(existingAttr.keySpan.end.offset)
128+
);
129+
} else {
130+
return (
131+
html.slice(0, existingAttr.sourceSpan.start.offset).trimEnd() +
132+
html.slice(existingAttr.sourceSpan.end.offset)
133+
);
134+
}
121135
}
122136
}
123137

124138
// Otherwise insert a new attribute.
139+
const newValue = update(null);
125140
const index = node.startSourceSpan.start.offset + node.name.length + 1;
126141
const prefix = html.slice(0, index);
127142
const suffix = html.slice(index);
128143

129144
if (node.startSourceSpan.start.line === node.startSourceSpan.end.line) {
130-
return prefix + ` ${name}="${value}"` + suffix;
145+
return prefix + ` ${name}="${newValue}"` + suffix;
131146
}
132147

133148
const attr = node.attributes[0];
134149
const ctx = attr.sourceSpan.start.getContext(attr.sourceSpan.start.col + 1, 1)!;
135150
const indentation = ctx.before;
136151

137-
return prefix + indentation + `${name}="${value}"` + suffix;
152+
return prefix + indentation + `${name}="${newValue}"` + suffix;
138153
}
139154

140155
/**

0 commit comments

Comments
 (0)