From a0414fddbc4297a7c6e3092572985ddc74fb28a4 Mon Sep 17 00:00:00 2001 From: Adam Plumer Date: Mon, 3 Jan 2022 00:12:05 -0600 Subject: [PATCH] feat(flex): detect display precedence in fxLayout directive In some cases, a user may intentionally hide a directive using styling that exists outside of the scope of this library. However, fxLayout had no way of detecting this, or establishing a precedence for pre-existing styles. This adds detection and special caching for pre-styled display attributes on an element. This means that if, at any point in the change detection of the directive for its entire lifespan, it detects that the style has been set to `display: none`, it will use that as the display value in place of what it otherwise would have used. Please note: this means that you need to be more careful when using this functionality! If, for instance, you have functionality that sets `display: none` on a directive, that functionality needs to now take over restoring the correct style for the directive. This is essentially an escape hatch from the library: saying that you know better how to manage these directives. To use this feature, do the following: ```ts @NgModule({ imports: [ FlexLayoutModule.withConfig({detectLayoutDisplay: true}), ], }) export class AppModule {} ``` Your markup should then adapt automatically to the new behavior. --- .../_private-utils/layout-validator.ts | 8 +- .../core/style-utils/style-utils.ts | 74 +++++++++---------- .../flex-layout/core/tokens/library-config.ts | 2 + .../flex-layout/flex/layout/layout.spec.ts | 10 ++- .../libs/flex-layout/flex/layout/layout.ts | 34 +++++++-- 5 files changed, 80 insertions(+), 48 deletions(-) diff --git a/projects/libs/flex-layout/_private-utils/layout-validator.ts b/projects/libs/flex-layout/_private-utils/layout-validator.ts index 885ee0a86..04c97837e 100644 --- a/projects/libs/flex-layout/_private-utils/layout-validator.ts +++ b/projects/libs/flex-layout/_private-utils/layout-validator.ts @@ -14,14 +14,14 @@ export const LAYOUT_VALUES = ['row', 'column', 'row-reverse', 'column-reverse']; export function buildLayoutCSS(value: string) { let [direction, wrap, isInline] = validateValue(value); return buildCSS(direction, wrap, isInline); - } +} /** * Validate the value to be one of the acceptable value options * Use default fallback of 'row' */ export function validateValue(value: string): [string, string, boolean] { - value = value ? value.toLowerCase() : ''; + value = value?.toLowerCase() ?? ''; let [direction, wrap, inline] = value.split(' '); // First value must be the `flex-direction` @@ -84,9 +84,9 @@ export function validateWrapValue(value: string) { */ function buildCSS(direction: string, wrap: string | null = null, inline = false) { return { - 'display': inline ? 'inline-flex' : 'flex', + display: inline ? 'inline-flex' : 'flex', 'box-sizing': 'border-box', 'flex-direction': direction, - 'flex-wrap': !!wrap ? wrap : null + 'flex-wrap': wrap || null, }; } diff --git a/projects/libs/flex-layout/core/style-utils/style-utils.ts b/projects/libs/flex-layout/core/style-utils/style-utils.ts index 983cbb47d..1afb65212 100644 --- a/projects/libs/flex-layout/core/style-utils/style-utils.ts +++ b/projects/libs/flex-layout/core/style-utils/style-utils.ts @@ -69,7 +69,7 @@ export class StyleUtils { * Find the DOM element's raw attribute value (if any) */ lookupAttributeValue(element: HTMLElement, attribute: string): string { - return element.getAttribute(attribute) || ''; + return element.getAttribute(attribute) ?? ''; } /** @@ -77,7 +77,7 @@ export class StyleUtils { */ lookupInlineStyle(element: HTMLElement, styleName: string): string { return isPlatformBrowser(this._platformId) ? - element.style.getPropertyValue(styleName) : this._getServerStyle(element, styleName); + element.style.getPropertyValue(styleName) : getServerStyle(element, styleName); } /** @@ -121,56 +121,56 @@ export class StyleUtils { value = value ? value + '' : ''; if (isPlatformBrowser(this._platformId) || !this._serverModuleLoaded) { isPlatformBrowser(this._platformId) ? - element.style.setProperty(key, value) : this._setServerStyle(element, key, value); + element.style.setProperty(key, value) : setServerStyle(element, key, value); } else { this._serverStylesheet.addStyleToElement(element, key, value); } } }); } +} - private _setServerStyle(element: any, styleName: string, styleValue?: string|null) { - styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); - const styleMap = this._readStyleAttribute(element); - styleMap[styleName] = styleValue || ''; - this._writeStyleAttribute(element, styleMap); - } +function getServerStyle(element: any, styleName: string): string { + const styleMap = readStyleAttribute(element); + return styleMap[styleName] ?? ''; +} - private _getServerStyle(element: any, styleName: string): string { - const styleMap = this._readStyleAttribute(element); - return styleMap[styleName] || ''; - } +function setServerStyle(element: any, styleName: string, styleValue?: string|null) { + styleName = styleName.replace(/([a-z])([A-Z])/g, '$1-$2').toLowerCase(); + const styleMap = readStyleAttribute(element); + styleMap[styleName] = styleValue ?? ''; + writeStyleAttribute(element, styleMap); +} - private _readStyleAttribute(element: any): {[name: string]: string} { - const styleMap: {[name: string]: string} = {}; - const styleAttribute = element.getAttribute('style'); - if (styleAttribute) { - const styleList = styleAttribute.split(/;+/g); - for (let i = 0; i < styleList.length; i++) { - const style = styleList[i].trim(); - if (style.length > 0) { - const colonIndex = style.indexOf(':'); - if (colonIndex === -1) { - throw new Error(`Invalid CSS style: ${style}`); - } - const name = style.substr(0, colonIndex).trim(); - styleMap[name] = style.substr(colonIndex + 1).trim(); - } - } +function writeStyleAttribute(element: any, styleMap: {[name: string]: string}) { + let styleAttrValue = ''; + for (const key in styleMap) { + const newValue = styleMap[key]; + if (newValue) { + styleAttrValue += `${key}:${styleMap[key]};`; } - return styleMap; } + element.setAttribute('style', styleAttrValue); +} - private _writeStyleAttribute(element: any, styleMap: {[name: string]: string}) { - let styleAttrValue = ''; - for (const key in styleMap) { - const newValue = styleMap[key]; - if (newValue) { - styleAttrValue += key + ':' + styleMap[key] + ';'; +function readStyleAttribute(element: any): {[name: string]: string} { + const styleMap: {[name: string]: string} = {}; + const styleAttribute = element.getAttribute('style'); + if (styleAttribute) { + const styleList = styleAttribute.split(/;+/g); + for (let i = 0; i < styleList.length; i++) { + const style = styleList[i].trim(); + if (style.length > 0) { + const colonIndex = style.indexOf(':'); + if (colonIndex === -1) { + throw new Error(`Invalid CSS style: ${style}`); + } + const name = style.substr(0, colonIndex).trim(); + styleMap[name] = style.substr(colonIndex + 1).trim(); } } - element.setAttribute('style', styleAttrValue); } + return styleMap; } /** diff --git a/projects/libs/flex-layout/core/tokens/library-config.ts b/projects/libs/flex-layout/core/tokens/library-config.ts index 10b7563f1..7cda4d13f 100644 --- a/projects/libs/flex-layout/core/tokens/library-config.ts +++ b/projects/libs/flex-layout/core/tokens/library-config.ts @@ -21,6 +21,7 @@ export interface LayoutConfigOptions { ssrObserveBreakpoints?: string[]; multiplier?: Multiplier; defaultUnit?: string; + detectLayoutDisplay?: boolean; } export const DEFAULT_CONFIG: Required = { @@ -38,6 +39,7 @@ export const DEFAULT_CONFIG: Required = { // Instead, we disable it by default, which requires this ugly cast. multiplier: undefined as unknown as Multiplier, defaultUnit: 'px', + detectLayoutDisplay: false, }; export const LAYOUT_CONFIG = new InjectionToken( diff --git a/projects/libs/flex-layout/flex/layout/layout.spec.ts b/projects/libs/flex-layout/flex/layout/layout.spec.ts index db7d0ffa4..0d1151b27 100644 --- a/projects/libs/flex-layout/flex/layout/layout.spec.ts +++ b/projects/libs/flex-layout/flex/layout/layout.spec.ts @@ -39,7 +39,7 @@ describe('layout directive', () => { // Configure testbed to prepare services TestBed.configureTestingModule({ - imports: [CommonModule, FlexLayoutModule], + imports: [CommonModule, FlexLayoutModule.withConfig({detectLayoutDisplay: true})], declarations: [TestLayoutComponent], providers: [ MockMatchMediaProvider, @@ -66,6 +66,14 @@ describe('layout directive', () => { 'box-sizing': 'border-box' }, styler); }); + it('should not override pre-existing styles', () => { + createTestComponent(`
`); + expectNativeEl(fixture).toHaveStyle({ + 'display': 'none', + 'flex-direction': 'row', + 'box-sizing': 'border-box' + }, styler); + }); it('should add correct styles for `fxLayout="row wrap"` usage', () => { createTestComponent(`
`); expectNativeEl(fixture).toHaveStyle({ diff --git a/projects/libs/flex-layout/flex/layout/layout.ts b/projects/libs/flex-layout/flex/layout/layout.ts index cbc145f39..b910c8a0f 100644 --- a/projects/libs/flex-layout/flex/layout/layout.ts +++ b/projects/libs/flex-layout/flex/layout/layout.ts @@ -5,21 +5,31 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {Directive, ElementRef, OnChanges, Injectable} from '@angular/core'; +import {Directive, ElementRef, OnChanges, Injectable, Inject} from '@angular/core'; import { BaseDirective2, StyleBuilder, StyleDefinition, StyleUtils, MediaMarshaller, + LAYOUT_CONFIG, + LayoutConfigOptions, } from '@angular/flex-layout/core'; import {buildLayoutCSS} from '@angular/flex-layout/_private-utils'; +export interface LayoutStyleDisplay { + readonly display: string; +} + @Injectable({providedIn: 'root'}) export class LayoutStyleBuilder extends StyleBuilder { - buildStyles(input: string) { - return buildLayoutCSS(input); + buildStyles(input: string, {display}: LayoutStyleDisplay) { + const css = buildLayoutCSS(input); + return { + ...css, + display: display === 'none' ? display : css.display, + }; } } @@ -51,12 +61,23 @@ export class LayoutDirective extends BaseDirective2 implements OnChanges { constructor(elRef: ElementRef, styleUtils: StyleUtils, styleBuilder: LayoutStyleBuilder, - marshal: MediaMarshaller) { + marshal: MediaMarshaller, + @Inject(LAYOUT_CONFIG) private _config: LayoutConfigOptions) { super(elRef, styleBuilder, styleUtils, marshal); this.init(); } - protected styleCache = layoutCache; + protected updateWithValue(input: string) { + const detectLayoutDisplay = this._config.detectLayoutDisplay; + const display = detectLayoutDisplay ? this.styler.lookupStyle(this.nativeElement, 'display') : ''; + this.styleCache = cacheMap.get(display) ?? new Map(); + cacheMap.set(display, this.styleCache); + + if (this.currentValue !== input) { + this.addStyles(input, {display}); + this.currentValue = input; + } + } } @Directive({selector, inputs}) @@ -64,4 +85,5 @@ export class DefaultLayoutDirective extends LayoutDirective { protected inputs = inputs; } -const layoutCache: Map = new Map(); +type CacheMap = Map; +const cacheMap = new Map();