diff --git a/packages/core/src/render3/node_selector_matcher.ts b/packages/core/src/render3/node_selector_matcher.ts index 065233674c..9fcc32cfa7 100644 --- a/packages/core/src/render3/node_selector_matcher.ts +++ b/packages/core/src/render3/node_selector_matcher.ts @@ -11,6 +11,7 @@ import '../util/ng_dev_mode'; import {assertDefined, assertNotEqual} from '../util/assert'; import {AttributeMarker, TAttributes, TNode, TNodeType, unusedValueExportToPlacateAjd as unused1} from './interfaces/node'; import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags, unusedValueExportToPlacateAjd as unused2} from './interfaces/projection'; +import {getInitialClassNameValue} from './styling/class_and_style_bindings'; const unusedValueToPlacateAjd = unused1 + unused2; @@ -58,7 +59,6 @@ function hasTagAndTypeMatch( export function isNodeMatchingSelector( tNode: TNode, selector: CssSelector, isProjectionMode: boolean): boolean { ngDevMode && assertDefined(selector[0], 'Selector should have a tag name'); - let mode: SelectorFlags = SelectorFlags.ELEMENT; const nodeAttrs = tNode.attrs !; const selectOnlyMarkerIdx = nodeAttrs ? nodeAttrs.indexOf(AttributeMarker.SelectOnly) : -1; @@ -92,7 +92,19 @@ export function isNodeMatchingSelector( skipToNextSelector = true; } } else { - const attrName = mode & SelectorFlags.CLASS ? 'class' : current; + const selectorAttrValue = mode & SelectorFlags.CLASS ? current : selector[++i]; + + // special case for matching against classes when a tNode has been instantiated with + // class and style values as separate attribute values (e.g. ['title', CLASS, 'foo']) + if ((mode & SelectorFlags.CLASS) && tNode.stylingTemplate) { + if (!isCssClassMatching(readClassValueFromTNode(tNode), selectorAttrValue as string)) { + if (isPositive(mode)) return false; + skipToNextSelector = true; + } + continue; + } + + const attrName = (mode & SelectorFlags.CLASS) ? 'class' : current; const attrIndexInNode = findAttrIndexInNode(attrName, nodeAttrs); if (attrIndexInNode === -1) { @@ -101,7 +113,6 @@ export function isNodeMatchingSelector( continue; } - const selectorAttrValue = mode & SelectorFlags.CLASS ? current : selector[++i]; if (selectorAttrValue !== '') { let nodeAttrValue: string; const maybeAttrName = nodeAttrs[attrIndexInNode]; @@ -113,8 +124,10 @@ export function isNodeMatchingSelector( 'We do not match directives on namespaced attributes'); nodeAttrValue = nodeAttrs[attrIndexInNode + 1] as string; } - if (mode & SelectorFlags.CLASS && - !isCssClassMatching(nodeAttrValue as string, selectorAttrValue as string) || + + const compareAgainstClassName = mode & SelectorFlags.CLASS ? nodeAttrValue : null; + if (compareAgainstClassName && + !isCssClassMatching(compareAgainstClassName, selectorAttrValue as string) || mode & SelectorFlags.ATTRIBUTE && selectorAttrValue !== nodeAttrValue) { if (isPositive(mode)) return false; skipToNextSelector = true; @@ -130,6 +143,16 @@ function isPositive(mode: SelectorFlags): boolean { return (mode & SelectorFlags.NOT) === 0; } +function readClassValueFromTNode(tNode: TNode): string { + // comparing against CSS class values is complex because the compiler doesn't place them as + // regular attributes when an element is created. Instead, the classes (and styles for + // that matter) are placed in a special styling context that is used for resolving all + // class/style values across static attributes, [style]/[class] and [style.prop]/[class.name] + // bindings. Therefore if and when the styling context exists then the class values are to be + // extracted by the context helper code below... + return tNode.stylingTemplate ? getInitialClassNameValue(tNode.stylingTemplate) : ''; +} + /** * Examines an attributes definition array from a node to find the index of the * attribute with the specified name. diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 3fa175255b..0de16a9c3f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -1022,6 +1022,9 @@ { "name": "queueComponentIndexForCheck" }, + { + "name": "readClassValueFromTNode" + }, { "name": "readElementValue" }, diff --git a/packages/core/test/linker/projection_integration_spec.ts b/packages/core/test/linker/projection_integration_spec.ts index d41a092e41..23bbb76eb9 100644 --- a/packages/core/test/linker/projection_integration_spec.ts +++ b/packages/core/test/linker/projection_integration_spec.ts @@ -81,22 +81,35 @@ describe('projection', () => { expect(main.nativeElement).toHaveText(''); }); - fixmeIvy('FW-833: Directive / projected node matching against class name') - .it('should support multiple content tags', () => { - TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]}); - TestBed.overrideComponent(MainComp, { - set: { - template: '' + - '
B
' + - '
C
' + - '
A
' + - '
' - } - }); - const main = TestBed.createComponent(MainComp); + it('should project a single class-based tag', () => { + TestBed.configureTestingModule({declarations: [SingleContentTagComponent]}); + TestBed.overrideComponent(MainComp, { + set: { + template: '' + + '
I AM PROJECTED
' + + '
' + } + }); + const main = TestBed.createComponent(MainComp); - expect(main.nativeElement).toHaveText('(A, BC)'); - }); + expect(main.nativeElement).toHaveText('I AM PROJECTED'); + }); + + fixmeIvy('unknown').it('should support multiple content tags', () => { + TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]}); + TestBed.overrideComponent(MainComp, { + set: { + template: '' + + '
B
' + + '
C
' + + '
A
' + + '
' + } + }); + const main = TestBed.createComponent(MainComp); + + expect(main.nativeElement).toHaveText('(A, BC)'); + }); it('should redistribute only direct children', () => { TestBed.configureTestingModule({declarations: [MultipleContentTagsComponent]}); @@ -182,35 +195,34 @@ describe('projection', () => { expect(main.nativeElement).toHaveText('OUTER(INNER(INNERINNER(A,BC)))'); }); - fixmeIvy('FW-833: Directive / projected node matching against class name') - .it('should redistribute when the shadow dom changes', () => { - TestBed.configureTestingModule( - {declarations: [ConditionalContentComponent, ManualViewportDirective]}); - TestBed.overrideComponent(MainComp, { - set: { - template: '' + - '
A
' + - '
B
' + - '
C
' + - '
' - } - }); - const main = TestBed.createComponent(MainComp); + fixmeIvy('unknown').it('should redistribute when the shadow dom changes', () => { + TestBed.configureTestingModule( + {declarations: [ConditionalContentComponent, ManualViewportDirective]}); + TestBed.overrideComponent(MainComp, { + set: { + template: '' + + '
A
' + + '
B
' + + '
C
' + + '
' + } + }); + const main = TestBed.createComponent(MainComp); - const viewportDirective = - main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( - ManualViewportDirective); + const viewportDirective = + main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( + ManualViewportDirective); - expect(main.nativeElement).toHaveText('(, BC)'); + expect(main.nativeElement).toHaveText('(, BC)'); - viewportDirective.show(); - main.detectChanges(); - expect(main.nativeElement).toHaveText('(A, BC)'); + viewportDirective.show(); + main.detectChanges(); + expect(main.nativeElement).toHaveText('(A, BC)'); - viewportDirective.hide(); - main.detectChanges(); - expect(main.nativeElement).toHaveText('(, BC)'); - }); + viewportDirective.hide(); + main.detectChanges(); + expect(main.nativeElement).toHaveText('(, BC)'); + }); // GH-2095 - https://github.com/angular/angular/issues/2095 // important as we are removing the ng-content element during compilation, @@ -290,38 +302,36 @@ describe('projection', () => { expect(main.nativeElement).toHaveText('SIMPLE()START(A)END'); }); - fixmeIvy('FW-833: Directive / projected node matching against class name') - .it('should support moving ng-content around', () => { - TestBed.configureTestingModule({ - declarations: [ConditionalContentComponent, ProjectDirective, ManualViewportDirective] - }); - TestBed.overrideComponent(MainComp, { - set: { - template: '' + - '
A
' + - '
B
' + - '
' + - 'START(
)END' - } - }); - const main = TestBed.createComponent(MainComp); + fixmeIvy('unknown').it('should support moving ng-content around', () => { + TestBed.configureTestingModule( + {declarations: [ConditionalContentComponent, ProjectDirective, ManualViewportDirective]}); + TestBed.overrideComponent(MainComp, { + set: { + template: '' + + '
A
' + + '
B
' + + '
' + + 'START(
)END' + } + }); + const main = TestBed.createComponent(MainComp); - const sourceDirective: ManualViewportDirective = - main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( - ManualViewportDirective); - const projectDirective: ProjectDirective = - main.debugElement.queryAllNodes(By.directive(ProjectDirective))[0].injector.get( - ProjectDirective); - expect(main.nativeElement).toHaveText('(, B)START()END'); + const sourceDirective: ManualViewportDirective = + main.debugElement.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( + ManualViewportDirective); + const projectDirective: ProjectDirective = + main.debugElement.queryAllNodes(By.directive(ProjectDirective))[0].injector.get( + ProjectDirective); + expect(main.nativeElement).toHaveText('(, B)START()END'); - projectDirective.show(sourceDirective.templateRef); - expect(main.nativeElement).toHaveText('(, B)START(A)END'); + projectDirective.show(sourceDirective.templateRef); + expect(main.nativeElement).toHaveText('(, B)START(A)END'); - // Stamping ng-content multiple times should not produce the content multiple - // times... - projectDirective.show(sourceDirective.templateRef); - expect(main.nativeElement).toHaveText('(, B)START(A)END'); - }); + // Stamping ng-content multiple times should not produce the content multiple + // times... + projectDirective.show(sourceDirective.templateRef); + expect(main.nativeElement).toHaveText('(, B)START(A)END'); + }); // Note: This does not use a ng-content element, but // is still important as we are merging proto views independent of @@ -533,49 +543,48 @@ describe('projection', () => { expect(main.nativeElement).toHaveText('B(A)'); }); - fixmeIvy('FW-833: Directive / projected node matching against class name') - .it('should project filled view containers into a view container', () => { - TestBed.configureTestingModule( - {declarations: [ConditionalContentComponent, ManualViewportDirective]}); - TestBed.overrideComponent(MainComp, { - set: { - template: '' + - '
A
' + - 'B' + - '
C
' + - '
D
' + - '
' - } - }); - const main = TestBed.createComponent(MainComp); + fixmeIvy('unknown').it('should project filled view containers into a view container', () => { + TestBed.configureTestingModule( + {declarations: [ConditionalContentComponent, ManualViewportDirective]}); + TestBed.overrideComponent(MainComp, { + set: { + template: '' + + '
A
' + + 'B' + + '
C
' + + '
D
' + + '
' + } + }); + const main = TestBed.createComponent(MainComp); - const conditionalComp = main.debugElement.query(By.directive(ConditionalContentComponent)); + const conditionalComp = main.debugElement.query(By.directive(ConditionalContentComponent)); - const viewViewportDir = - conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( - ManualViewportDirective); + const viewViewportDir = + conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[0].injector.get( + ManualViewportDirective); - expect(main.nativeElement).toHaveText('(, D)'); - expect(main.nativeElement).toHaveText('(, D)'); + expect(main.nativeElement).toHaveText('(, D)'); + expect(main.nativeElement).toHaveText('(, D)'); - viewViewportDir.show(); - main.detectChanges(); - expect(main.nativeElement).toHaveText('(AC, D)'); + viewViewportDir.show(); + main.detectChanges(); + expect(main.nativeElement).toHaveText('(AC, D)'); - const contentViewportDir = - conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[1].injector.get( - ManualViewportDirective); + const contentViewportDir = + conditionalComp.queryAllNodes(By.directive(ManualViewportDirective))[1].injector.get( + ManualViewportDirective); - contentViewportDir.show(); - main.detectChanges(); - expect(main.nativeElement).toHaveText('(ABC, D)'); + contentViewportDir.show(); + main.detectChanges(); + expect(main.nativeElement).toHaveText('(ABC, D)'); - // hide view viewport, and test that it also hides - // the content viewport's views - viewViewportDir.hide(); - main.detectChanges(); - expect(main.nativeElement).toHaveText('(, D)'); - }); + // hide view viewport, and test that it also hides + // the content viewport's views + viewViewportDir.hide(); + main.detectChanges(); + expect(main.nativeElement).toHaveText('(, D)'); + }); }); @Component({selector: 'main', template: ''}) @@ -626,6 +635,13 @@ class Empty { class MultipleContentTagsComponent { } +@Component({ + selector: 'single-content-tag', + template: '', +}) +class SingleContentTagComponent { +} + @Directive({selector: '[manual]'}) class ManualViewportDirective { constructor(public vc: ViewContainerRef, public templateRef: TemplateRef) {} diff --git a/packages/core/test/render3/node_selector_matcher_spec.ts b/packages/core/test/render3/node_selector_matcher_spec.ts index 5974dd4391..822dcca3ac 100644 --- a/packages/core/test/render3/node_selector_matcher_spec.ts +++ b/packages/core/test/render3/node_selector_matcher_spec.ts @@ -10,6 +10,7 @@ import {AttributeMarker, TAttributes, TNode, TNodeType} from '../../src/render3/ import {CssSelector, CssSelectorList, NG_PROJECT_AS_ATTR_NAME, SelectorFlags,} from '../../src/render3/interfaces/projection'; import {getProjectAsAttrValue, isNodeMatchingSelectorList, isNodeMatchingSelector} from '../../src/render3/node_selector_matcher'; +import {initializeStaticContext} from '../../src/render3/styling/class_and_style_bindings'; import {createTNode} from '@angular/core/src/render3/instructions'; import {getLView} from '@angular/core/src/render3/state'; @@ -18,9 +19,12 @@ function testLStaticData(tagName: string, attrs: TAttributes | null): TNode { } describe('css selector matching', () => { - function isMatching(tagName: string, attrs: TAttributes | null, selector: CssSelector): boolean { - return isNodeMatchingSelector( - createTNode(getLView(), TNodeType.Element, 0, tagName, attrs, null), selector, false); + function isMatching( + tagName: string, attrsOrTNode: TAttributes | TNode | null, selector: CssSelector): boolean { + const tNode = (!attrsOrTNode || Array.isArray(attrsOrTNode)) ? + createTNode(getLView(), TNodeType.Element, 0, tagName, attrsOrTNode as TAttributes, null) : + (attrsOrTNode as TNode); + return isNodeMatchingSelector(tNode, selector, false); } describe('isNodeMatchingSimpleSelector', () => { @@ -298,6 +302,26 @@ describe('css selector matching', () => { //
expect(isMatching('div', ['class', 'foo'], selector)).toBeFalsy(); }); + + it('should match against a class value before and after the styling context is created', + () => { + // selector: 'div.abc' + const selector = ['div', SelectorFlags.CLASS, 'abc']; + const tNode = createTNode(getLView(), TNodeType.Element, 0, 'div', [], null); + + //
(without attrs or styling context) + expect(isMatching('div', tNode, selector)).toBeFalsy(); + + //
(with attrs but without styling context) + tNode.attrs = ['class', 'abc']; + tNode.stylingTemplate = null; + expect(isMatching('div', tNode, selector)).toBeTruthy(); + + //
(with styling context but without attrs) + tNode.stylingTemplate = initializeStaticContext([AttributeMarker.Classes, 'abc']); + tNode.attrs = null; + expect(isMatching('div', tNode, selector)).toBeTruthy(); + }); }); });