Skip to content

feat(map-control): add style prop - #1052

Open
samithahansaka wants to merge 1 commit into
visgl:mainfrom
samithahansaka:feat/map-control-style
Open

feat(map-control): add style prop#1052
samithahansaka wants to merge 1 commit into
visgl:mainfrom
samithahansaka:feat/map-control-style

Conversation

@samithahansaka

Copy link
Copy Markdown
Contributor

Adds a style prop to MapControl, applied to the same container element that already receives className.

Closes #379. The className half of that issue shipped in #967, so this is the remaining part.

Motivation

The original request was to let a control break out of the default layout, for example {inset: '0px 0px auto 0px'} to make it span the full width of the map. Without this, the container the control renders into cannot be styled at all from React, because it is created imperatively and handed to the Maps JavaScript API.

Implementation

It follows the pattern already used by InfoWindow and Popover:

  • setValueForStyles from src/libraries/set-value-for-styles.ts, with a prevStyleRef holding the previously applied styles
  • inside a useLayoutEffect, so the control is not painted unstyled for a frame
  • className moved into the same effect, matching info-window.tsx

Using the shared helper means the prop behaves exactly like React's style everywhere else in the library: numbers get an implicit px suffix, CSS custom properties work, and falsy values unset a property. It also only touches the properties actually passed in, so inline styles the Maps API writes on the container are left alone.

Tests

Five new tests covering application, updates, removal of properties dropped between renders, px suffixing, custom properties, and that unrelated inline styles survive an update.

npm test and npm run build pass.

Two things worth discussing

Interaction with control layout. You raised this in the issue: controls registered at the same position are laid out by the API, so styles that change size or placement will affect neighbouring controls. I documented that caveat rather than trying to prevent it, since preventing it would mean deciding which properties are allowed. Happy to restrict the prop if you would rather.

Two eslint-disable comments. react-hooks/immutability reports the container mutation twice, once for the indirect write through setValueForStyles and once for the direct className assignment, so both needed suppressing. If there is a preferred way to structure this so the rule is satisfied without the second comment, I am glad to change it.

Allows inline styles to be applied to the control container, so a control
can break out of the default layout, for example spanning the full width
of the map with `inset: 0px 0px auto 0px`.

The styles are compared by value, since callers typically pass an inline
object literal that would otherwise be a new reference on every render.
Properties dropped between renders are removed from the element rather
than left behind.

Refs visgl#379
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add className and style prop to the MapControl component

1 participant