Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(widgets): convert widget style prop keys from camel to dash case #8991

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/api-reference/widgets/overview.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions modules/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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} from './utils/apply-styles';

// Types
export type {CoordinateSystem} from './lib/constants';
Expand Down
13 changes: 13 additions & 0 deletions modules/core/src/utils/apply-styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
export function applyStyles(element: HTMLElement, style?: Partial<CSSStyleDeclaration>): 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;
}
});
}
}
11 changes: 7 additions & 4 deletions modules/widgets/src/compass-widget.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
/* global document */
import {FlyToInterpolator, WebMercatorViewport, _GlobeViewport} from '@deck.gl/core';
import {
FlyToInterpolator,
WebMercatorViewport,
_GlobeViewport,
_applyStyles as applyStyles
} from '@deck.gl/core';
import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';

Expand Down Expand Up @@ -61,9 +66,7 @@ export class CompassWidget implements Widget<CompassWidgetProps> {
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();
Expand Down
22 changes: 12 additions & 10 deletions modules/widgets/src/fullscreen-widget.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* global document */
import {_deepEqual as deepEqual} from '@deck.gl/core';
import {_deepEqual as deepEqual, _applyStyles as applyStyles} from '@deck.gl/core';
import type {Deck, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';
import {IconButton} from './components';
Expand Down Expand Up @@ -55,9 +55,7 @@ export class FullscreenWidget implements Widget<FullscreenWidgetProps> {
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();
Expand Down Expand Up @@ -99,13 +97,17 @@ export class FullscreenWidget implements Widget<FullscreenWidgetProps> {

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)
);
Object.entries(oldProps.style).map(([key]) => {
if (key.startsWith('--')) {
// Assume CSS variable
el.style.removeProperty(key);
} else {
// Assume camelCase
el.style[key] = '';
}
});
Comment on lines +100 to +108
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's create a removeStyles as well. This seems useful to apply in all widgets when style has changed

}
applyStyles(el, props.style);
}
}

Expand Down
6 changes: 2 additions & 4 deletions modules/widgets/src/zoom-widget.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* global document */
import {FlyToInterpolator} from '@deck.gl/core';
import {FlyToInterpolator, _applyStyles as applyStyles} from '@deck.gl/core';
import type {Deck, Viewport, Widget, WidgetPlacement} from '@deck.gl/core';
import {render} from 'preact';
import {ButtonGroup, GroupedIconButton} from './components';
Expand Down Expand Up @@ -64,9 +64,7 @@ export class ZoomWidget implements Widget<ZoomWidgetProps> {
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 = (
<ButtonGroup orientation={this.orientation}>
<GroupedIconButton
Expand Down
30 changes: 30 additions & 0 deletions test/modules/core/utils/apply-styles.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
/* global document */
import test from 'tape-promise/tape';
import {applyStyles} from '@deck.gl/core/utils/apply-styles';

const TEST_CASES = [
{
title: 'CSS variable',
argument: {property: '--my-var', value: 'red'},
result: {property: '--my-var', value: 'red'}
},
{
title: 'camelCase property',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please test the faulty Kebab-case as well? I wonder what our runner will do

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 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();
});
1 change: 1 addition & 0 deletions test/modules/core/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,4 @@ import './range.spec';
import './math-utils.spec';
import './shader.spec';
import './typed-array-manager.spec';
import './apply-styles.spec';
Loading