From c1b59779fc2bee4353a5e6ebdeb4aa7c283de8e8 Mon Sep 17 00:00:00 2001 From: Tyler Matteo Date: Mon, 16 Dec 2024 19:06:48 -0500 Subject: [PATCH] fix(widgets): widget style prop keys should accept camelCase css properties and dashed css variables (#8991) * fix(widgets): handle css variable and camel cased keys passed to widget style prop --------- Co-authored-by: Chris Gervang --- docs/api-reference/widgets/compass-widget.md | 2 +- .../widgets/fullscreen-widget.md | 2 +- docs/api-reference/widgets/overview.md | 6 ++ docs/api-reference/widgets/zoom-widget.md | 2 +- modules/core/src/index.ts | 1 + modules/core/src/utils/apply-styles.ts | 27 +++++++++ modules/widgets/src/compass-widget.tsx | 27 +++++++-- modules/widgets/src/fullscreen-widget.tsx | 20 +++---- modules/widgets/src/zoom-widget.tsx | 25 ++++++-- test/modules/core/utils/apply-styles.spec.ts | 59 +++++++++++++++++++ test/modules/core/utils/index.ts | 1 + 11 files changed, 149 insertions(+), 23 deletions(-) create mode 100644 modules/core/src/utils/apply-styles.ts create mode 100644 test/modules/core/utils/apply-styles.spec.ts diff --git a/docs/api-reference/widgets/compass-widget.md b/docs/api-reference/widgets/compass-widget.md index 1fbdac151de..f59adc4c064 100644 --- a/docs/api-reference/widgets/compass-widget.md +++ b/docs/api-reference/widgets/compass-widget.md @@ -38,7 +38,7 @@ Bearing and pitch reset transition duration in milliseconds. Default: `{}` -Additional CSS styles for the canvas. +Additional CSS styles for the widget. camelCase CSS properties (e.g. `backgroundColor`) and kabab-case CSS variables are accepted (e.g. `--button-size`). #### `className` (string, optional) {#classname} diff --git a/docs/api-reference/widgets/fullscreen-widget.md b/docs/api-reference/widgets/fullscreen-widget.md index a1db2a65f04..40132108e39 100644 --- a/docs/api-reference/widgets/fullscreen-widget.md +++ b/docs/api-reference/widgets/fullscreen-widget.md @@ -38,7 +38,7 @@ Default: `'Exit Fullscreen'` Default: `{}` -Additional CSS styles for the canvas. +Additional CSS styles for the widget. camelCase CSS properties (e.g. `backgroundColor`) and kabab-case CSS variables are accepted (e.g. `--button-size`). #### `className` (string, optional) {#classname} diff --git a/docs/api-reference/widgets/overview.md b/docs/api-reference/widgets/overview.md index e55d3a34a7d..8a153a4ccd2 100644 --- a/docs/api-reference/widgets/overview.md +++ b/docs/api-reference/widgets/overview.md @@ -75,6 +75,12 @@ Apply styles to a single instance of a widget using inline styles. new FullscreenWidget({ style: {'--button-size': '48px'}}) ``` +To style hyphenated CSS properties (e.g. `background-color`, `border-color`, etc.), use the camelCase equivalent. + +```js +new FullscreenWidget({ style: {'backgroundColor': '#fff'}}) +``` + ### Custom Class Theming Define a custom class with your desired styles and apply it to a widget. diff --git a/docs/api-reference/widgets/zoom-widget.md b/docs/api-reference/widgets/zoom-widget.md index 5f8c8545e4b..5d718128bcc 100644 --- a/docs/api-reference/widgets/zoom-widget.md +++ b/docs/api-reference/widgets/zoom-widget.md @@ -50,7 +50,7 @@ Zoom transition duration in milliseconds. Default: `{}` -Additional CSS styles for the canvas. +Additional CSS styles for the widget. camelCase CSS properties (e.g. `backgroundColor`) and kabab-case CSS variables are accepted (e.g. `--button-size`). #### `className` (string, optional) {#classname} diff --git a/modules/core/src/index.ts b/modules/core/src/index.ts index 27cc54a8493..c79656a6e6e 100644 --- a/modules/core/src/index.ts +++ b/modules/core/src/index.ts @@ -107,6 +107,7 @@ export {deepEqual as _deepEqual} from './utils/deep-equal'; export {default as _memoize} from './utils/memoize'; export {mergeShaders as _mergeShaders} from './utils/shader'; export {compareProps as _compareProps} from './lifecycle/props'; +export {applyStyles as _applyStyles, removeStyles as _removeStyles} from './utils/apply-styles'; // Types export type {CoordinateSystem} from './lib/constants'; diff --git a/modules/core/src/utils/apply-styles.ts b/modules/core/src/utils/apply-styles.ts new file mode 100644 index 00000000000..35866203b4a --- /dev/null +++ b/modules/core/src/utils/apply-styles.ts @@ -0,0 +1,27 @@ +export function applyStyles(element: HTMLElement, style?: Partial): void { + if (style) { + Object.entries(style).map(([key, value]) => { + if (key.startsWith('--')) { + // Assume CSS variable + element.style.setProperty(key, value as string); + } else { + // Assume camelCase + element.style[key] = value; + } + }); + } +} + +export function removeStyles(element: HTMLElement, style?: Partial): void { + if (style) { + Object.keys(style).map(key => { + if (key.startsWith('--')) { + // Assume CSS variable + element.style.removeProperty(key); + } else { + // Assume camelCase + element.style[key] = ''; + } + }); + } +} diff --git a/modules/widgets/src/compass-widget.tsx b/modules/widgets/src/compass-widget.tsx index a81840c39a4..dff1bbd6c06 100644 --- a/modules/widgets/src/compass-widget.tsx +++ b/modules/widgets/src/compass-widget.tsx @@ -1,5 +1,12 @@ /* global document */ -import {FlyToInterpolator, WebMercatorViewport, _GlobeViewport} from '@deck.gl/core'; +import { + FlyToInterpolator, + WebMercatorViewport, + _GlobeViewport, + _deepEqual as deepEqual, + _applyStyles as applyStyles, + _removeStyles as removeStyles +} from '@deck.gl/core'; import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core'; import {render} from 'preact'; @@ -48,6 +55,20 @@ export class CompassWidget implements Widget { } setProps(props: Partial) { + const oldProps = this.props; + const el = this.element; + if (el) { + if (oldProps.className !== props.className) { + if (oldProps.className) el.classList.remove(oldProps.className); + if (props.className) el.classList.add(props.className); + } + + if (!deepEqual(oldProps.style, props.style, 1)) { + removeStyles(el, oldProps.style); + applyStyles(el, props.style); + } + } + Object.assign(this.props, props); } @@ -61,9 +82,7 @@ export class CompassWidget implements Widget { const element = document.createElement('div'); element.classList.add('deck-widget', 'deck-widget-compass'); if (className) element.classList.add(className); - if (style) { - Object.entries(style).map(([key, value]) => element.style.setProperty(key, value as string)); - } + applyStyles(element, style); this.deck = deck; this.element = element; this.update(); diff --git a/modules/widgets/src/fullscreen-widget.tsx b/modules/widgets/src/fullscreen-widget.tsx index 2829a7f4ec5..7f0c1ba69da 100644 --- a/modules/widgets/src/fullscreen-widget.tsx +++ b/modules/widgets/src/fullscreen-widget.tsx @@ -1,5 +1,9 @@ /* global document */ -import {_deepEqual as deepEqual} from '@deck.gl/core'; +import { + _deepEqual as deepEqual, + _applyStyles as applyStyles, + _removeStyles as removeStyles +} from '@deck.gl/core'; import type {Deck, Widget, WidgetPlacement} from '@deck.gl/core'; import {render} from 'preact'; import {IconButton} from './components'; @@ -55,9 +59,7 @@ export class FullscreenWidget implements Widget { const el = document.createElement('div'); el.classList.add('deck-widget', 'deck-widget-fullscreen'); if (className) el.classList.add(className); - if (style) { - Object.entries(style).map(([key, value]) => el.style.setProperty(key, value as string)); - } + applyStyles(el, style); this.deck = deck; this.element = el; this.update(); @@ -98,14 +100,8 @@ export class FullscreenWidget implements Widget { } if (!deepEqual(oldProps.style, props.style, 1)) { - if (oldProps.style) { - Object.entries(oldProps.style).map(([key]) => el.style.removeProperty(key)); - } - if (props.style) { - Object.entries(props.style).map(([key, value]) => - el.style.setProperty(key, value as string) - ); - } + removeStyles(el, oldProps.style); + applyStyles(el, props.style); } } diff --git a/modules/widgets/src/zoom-widget.tsx b/modules/widgets/src/zoom-widget.tsx index a2bea8af597..2a3e80f97d7 100644 --- a/modules/widgets/src/zoom-widget.tsx +++ b/modules/widgets/src/zoom-widget.tsx @@ -1,5 +1,10 @@ /* global document */ -import {FlyToInterpolator} from '@deck.gl/core'; +import { + FlyToInterpolator, + _deepEqual as deepEqual, + _applyStyles as applyStyles, + _removeStyles as removeStyles +} from '@deck.gl/core'; import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core'; import {render} from 'preact'; import {ButtonGroup, GroupedIconButton} from './components'; @@ -64,9 +69,7 @@ export class ZoomWidget implements Widget { const element = document.createElement('div'); element.classList.add('deck-widget', 'deck-widget-zoom'); if (className) element.classList.add(className); - if (style) { - Object.entries(style).map(([key, value]) => element.style.setProperty(key, value as string)); - } + applyStyles(element, style); const ui = ( { } setProps(props: Partial) { + const oldProps = this.props; + const el = this.element; + if (el) { + if (oldProps.className !== props.className) { + if (oldProps.className) el.classList.remove(oldProps.className); + if (props.className) el.classList.add(props.className); + } + + if (!deepEqual(oldProps.style, props.style, 1)) { + removeStyles(el, oldProps.style); + applyStyles(el, props.style); + } + } + Object.assign(this.props, props); } diff --git a/test/modules/core/utils/apply-styles.spec.ts b/test/modules/core/utils/apply-styles.spec.ts new file mode 100644 index 00000000000..e2fc45a17eb --- /dev/null +++ b/test/modules/core/utils/apply-styles.spec.ts @@ -0,0 +1,59 @@ +/* global document */ +import test from 'tape-promise/tape'; +import {applyStyles, removeStyles} from '@deck.gl/core/utils/apply-styles'; + +const APPLY_TEST_CASES = [ + { + title: 'CSS variable', + argument: {property: '--my-var', value: 'red'}, + result: {property: '--my-var', value: 'red'} + }, + { + title: 'camelCase property', + argument: {property: 'backgroundColor', value: 'red'}, + result: {property: 'background-color', value: 'red'} + } +]; + +test('applyStyles', t => { + const container = document.body.appendChild(document.createElement('div')); + for (const tc of APPLY_TEST_CASES) { + const {argument, result} = tc; + applyStyles(container, {[argument.property]: argument.value}); + t.deepEqual( + container.style.getPropertyValue(result.property), + result.value, + `applyStyles ${tc.title} returned expected result` + ); + } + t.end(); +}); + +const REMOVE_TEST_CASES = [ + { + title: 'CSS variable', + argument: {property: '--my-var', value: 'red'}, + result: {property: '--my-var', value: ''} + }, + { + title: 'camelCase property', + argument: {property: 'backgroundColor', value: 'red'}, + result: {property: 'background-color', value: ''} + } +]; + +test('removeStyles', t => { + const container = document.body.appendChild(document.createElement('div')); + for (const tc of REMOVE_TEST_CASES) { + const {argument, result} = tc; + // setProperty expects kabab-case + container.style.setProperty(result.property, argument.value); + removeStyles(container, {[argument.property]: argument.value}); + t.deepEqual( + container.style.getPropertyValue(result.property), + result.value, + `removeStyles ${tc.title} returned expected result` + ); + } + t.end(); +}); diff --git a/test/modules/core/utils/index.ts b/test/modules/core/utils/index.ts index 67724efbe6e..f8a227ef8d3 100644 --- a/test/modules/core/utils/index.ts +++ b/test/modules/core/utils/index.ts @@ -29,3 +29,4 @@ import './range.spec'; import './math-utils.spec'; import './shader.spec'; import './typed-array-manager.spec'; +import './apply-styles.spec';