-
Notifications
You must be signed in to change notification settings - Fork 78
[WC-3450] Complete LeafletMap MobX migration with ViewModel #2276
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
Open
iobuhov
wants to merge
16
commits into
3429/migrate-maps-to-mobx
Choose a base branch
from
3429/complete-mobx-migration
base: 3429/migrate-maps-to-mobx
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
84edec1
feat(maps): complete MobX migration and replace react-leaflet
claude 30e204e
refactor: make prop static
iobuhov 8f1b44e
fix: resolve api key issues
iobuhov a169f14
feat: add atoms
iobuhov 021111b
feat: sync specs
iobuhov ac480a8
chore: await fo api key
iobuhov 8c70c5b
feat(maps-web): complete leaflet mobx migration with viewmodel, extra…
iobuhov a8ed724
chore(maps-web): move openspec changes from root to maps-web package
iobuhov 3f1b7b6
Revert "chore(maps-web): move openspec changes from root to maps-web …
iobuhov 72d417f
chore: remove duplicate maps openspec changes from root (already arch…
iobuhov 91f3dc3
refactor(maps-web): inline tile layer logic in LeafletMapViewModel, r…
iobuhov e9592f5
refactor(maps-web): use React 18 useEffect pattern for LeafletMap ref…
iobuhov c36a9ad
refactor(maps-web): extract tile layer config from ViewModel to utils…
iobuhov 8ca3fab
refactor(maps-web): use ref callback with disposeMap instead of useEf…
iobuhov fec1308
refactor(maps-web): move tile layer config to private method, remove …
iobuhov 1beda58
chore(maps-web): archive complete-mobx-migration openspec change
iobuhov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
57 changes: 57 additions & 0 deletions
57
...idgets/maps-web/openspec/changes/archive/2026-06-17-maps-api-key-atom/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| ## Context | ||
|
|
||
| Currently `MapsConfig.apiKey` is set once at container creation: `props.apiKeyExp?.value ?? props.apiKey`. Since `apiKeyExp` is a `DynamicValue<string>`, its `.value` can be `undefined` on the first render and resolve later. The static snapshot misses this. | ||
|
|
||
| The datagrid widget uses `ComputedAtom<T>` (from `@mendix/widget-plugin-mobx-kit`) for reactive derived values in the DI container. Pattern: a function that returns `computed(() => ...)`, registered as a constant binding. | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - API key resolved reactively from `mainGate.props` | ||
| - Priority: `apiKeyExp?.value` > `apiKey` > `null` | ||
| - Once a non-null value is observed, it's cached permanently | ||
| - Atom registered in DI container via a token, consumed by services | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - Changing how the key is used downstream (geocoding, tile layers still receive `string | undefined`) | ||
| - Making `geodecodeApiKey` an atom (separate concern, can follow same pattern later) | ||
|
|
||
| ## Decisions | ||
|
|
||
| **1. Use `ComputedAtom<string | null>` with closure-based caching** | ||
|
|
||
| A plain closure variable caches the first non-null result. Once set, the computed short-circuits without accessing `gate.props`, so MobX drops the dependency and the atom never re-evaluates. | ||
|
|
||
| ```ts | ||
| function apiKeyAtom(gate: DerivedPropsGate<MapsContainerProps>): ComputedAtom<string | null> { | ||
| let cached: string | null = null; | ||
| return computed(() => { | ||
| if (cached !== null) return cached; | ||
| const value = (gate.props.apiKeyExp?.value ?? gate.props.apiKey) || null; | ||
| if (value) cached = value; | ||
| return value; | ||
| }); | ||
| } | ||
| ``` | ||
|
|
||
| Alternative considered: `observable.box` + `runInAction`. Rejected — unnecessary complexity; a plain variable achieves the same "cache forever" behavior because MobX naturally stops tracking deps that aren't read. | ||
|
|
||
| **2. Register as `CORE.apiKey` token** | ||
|
|
||
| Add `apiKey: token<ComputedAtom<string | null>>(label("apiKey"))` to `CORE_TOKENS`. Bind in container init phase since it depends on `mainGate`. | ||
|
|
||
| **3. Remove `apiKey` from `MapsConfig`** | ||
|
|
||
| The static config no longer holds the key. `MapsConfig` keeps `id`, `name`, `showCurrentLocation`. | ||
|
|
||
| **4. Update consumers** | ||
|
|
||
| - `LocationResolverService.apiKey` computed → reads from injected atom `.get()` | ||
| - `MapsWidget.tsx` `mapsToken` prop → reads from atom via hook or passes through from LocationResolver (depends on whether view needs it directly) | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **[Closure mutation inside computed]** → Writing to a plain variable inside a computed is safe because MobX only tracks observable reads, not plain variable writes. The write is idempotent (set once, never again). | ||
| - **[Null initial state]** → Downstream consumers must handle `null`. The tile layer and geocoding already handle undefined keys gracefully (no-op until key arrives). |
29 changes: 29 additions & 0 deletions
29
...gets/maps-web/openspec/changes/archive/2026-06-17-maps-api-key-atom/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| ## Why | ||
|
|
||
| The `apiKey` is currently stored as a static field in `MapsConfig`, snapshot at container creation time. Since `apiKeyExp` is a `DynamicValue<string>` that may not be resolved on first render, the config can lock in `undefined` and miss the actual key. The key needs to be a reactive computed atom that resolves lazily and caches once available. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - Remove `apiKey` from `MapsConfig` (static config object) | ||
| - Create an `apiKeyAtom` as a `ComputedAtom<string | null>` registered in the DI container | ||
| - The atom prioritizes `apiKeyExp?.value`, falls back to `apiKey` (static), returns `null` when neither is available | ||
| - Once a non-null value is observed, the atom caches it permanently (never reverts to null) | ||
| - Update `LocationResolverService` to consume the atom instead of reading `mainGate.props` directly for the API key | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `api-key-atom`: Reactive, cached API key resolution via a MobX computed atom in the Maps DI container | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| _(none)_ | ||
|
|
||
| ## Impact | ||
|
|
||
| - `src/model/configs/Maps.config.ts` — remove `apiKey` field | ||
| - `src/model/tokens.ts` — add token for apiKey atom | ||
| - `src/model/containers/Maps.container.ts` — bind the atom | ||
| - `src/model/services/LocationResolver.service.ts` — use atom instead of `mainGate.props` for apiKey | ||
| - `src/components/MapsWidget.tsx` — remove `mapsToken` prop derivation (now handled by atom) |
79 changes: 79 additions & 0 deletions
79
...penspec/changes/archive/2026-06-17-maps-api-key-atom/specs/api-key-atom/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: API key resolved via computed atom | ||
|
|
||
| The Maps container SHALL provide a `ComputedAtom<string | null>` that reactively resolves the API key from widget props. | ||
|
|
||
| #### Scenario: Expression value takes priority | ||
|
|
||
| - **WHEN** `apiKeyExp.value` is a non-empty string | ||
| - **THEN** the atom returns that value | ||
|
|
||
| #### Scenario: Falls back to static apiKey | ||
|
|
||
| - **WHEN** `apiKeyExp.value` is undefined or empty | ||
| - **AND** `apiKey` is a non-empty string | ||
| - **THEN** the atom returns the static `apiKey` value | ||
|
|
||
| #### Scenario: Returns null when no key available | ||
|
|
||
| - **WHEN** both `apiKeyExp.value` and `apiKey` are empty or undefined | ||
| - **THEN** the atom returns `null` | ||
|
|
||
| ### Requirement: API key cached once resolved | ||
|
|
||
| Once the atom returns a non-null value, it SHALL cache that value permanently and never revert to `null`. | ||
|
|
||
| #### Scenario: Key remains after expression becomes unavailable | ||
|
|
||
| - **WHEN** the atom has previously returned a non-null value | ||
| - **AND** `apiKeyExp.value` subsequently becomes undefined | ||
| - **THEN** the atom still returns the previously cached value | ||
|
|
||
| ### Requirement: API key atom registered in DI container | ||
|
|
||
| The atom SHALL be registered as a `CORE_TOKENS.apiKey` token in the Maps container and injectable into services. | ||
|
|
||
| #### Scenario: LocationResolverService uses atom | ||
|
|
||
| - **WHEN** `LocationResolverService` needs the API key for geocoding | ||
| - **THEN** it reads from the injected `ComputedAtom<string | null>` via `.get()` | ||
|
|
||
| ### Requirement: Geodecode API key resolved via computed atom | ||
|
|
||
| The Maps container SHALL provide a `ComputedAtom<string | null>` that reactively resolves the geodecode API key from widget props, following the same pattern as the main API key atom. | ||
|
|
||
| #### Scenario: Expression value takes priority | ||
|
|
||
| - **WHEN** `geodecodeApiKeyExp.value` is a non-empty string | ||
| - **THEN** the atom returns that value | ||
|
|
||
| #### Scenario: Falls back to static geodecodeApiKey | ||
|
|
||
| - **WHEN** `geodecodeApiKeyExp.value` is undefined or empty | ||
| - **AND** `geodecodeApiKey` is a non-empty string | ||
| - **THEN** the atom returns the static `geodecodeApiKey` value | ||
|
|
||
| #### Scenario: Returns null when no key available | ||
|
|
||
| - **WHEN** both `geodecodeApiKeyExp.value` and `geodecodeApiKey` are empty or undefined | ||
| - **THEN** the atom returns `null` | ||
|
|
||
| ### Requirement: Geodecode API key cached once resolved | ||
|
|
||
| Once the geodecode atom returns a non-null value, it SHALL cache that value permanently and never revert to `null`. | ||
|
|
||
| #### Scenario: Key remains after expression becomes unavailable | ||
|
|
||
| - **WHEN** the atom has previously returned a non-null value | ||
| - **AND** `geodecodeApiKeyExp.value` subsequently becomes undefined | ||
| - **THEN** the atom still returns the previously cached value | ||
|
|
||
| ### Requirement: apiKey and geodecodeApiKey removed from MapsConfig | ||
|
|
||
| The static `MapsConfig` interface SHALL NOT contain `apiKey` or `geodecodeApiKey` fields. Both keys are resolved reactively via atoms. | ||
|
|
||
| #### Scenario: MapsConfig only contains static fields | ||
|
|
||
| - **WHEN** `mapsConfig()` is called | ||
| - **THEN** the returned object contains `id`, `name`, and `showCurrentLocation` only | ||
26 changes: 26 additions & 0 deletions
26
...Widgets/maps-web/openspec/changes/archive/2026-06-17-maps-api-key-atom/tasks.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| ## 1. Create the key atoms | ||
|
|
||
| - [x] 1.1 Create `src/model/atoms/apiKey.atom.ts` with `apiKeyAtom` function that returns `ComputedAtom<string | null>` with caching logic | ||
| - [x] 1.2 Create `src/model/atoms/geodecodeApiKey.atom.ts` with `geodecodeApiKeyAtom` function (same pattern, reads `geodecodeApiKeyExp?.value ?? geodecodeApiKey`) | ||
| - [x] 1.3 Add `apiKey: token<ComputedAtom<string | null>>` and `geodecodeApiKey: token<ComputedAtom<string | null>>` to `CORE_TOKENS` in `src/model/tokens.ts` | ||
|
|
||
| ## 2. Update MapsConfig | ||
|
|
||
| - [x] 2.1 Remove `apiKey` field from `MapsConfig` interface and `mapsConfig()` function | ||
| - [x] 2.2 Update `createMapsContainer.ts` if it references config.apiKey | ||
|
|
||
| ## 3. Wire atoms in container | ||
|
|
||
| - [x] 3.1 Bind both atoms in `Maps.container.ts` init phase (need mainGate): `CORE.apiKey` and `CORE.geodecodeApiKey` | ||
|
|
||
| ## 4. Update consumers | ||
|
|
||
| - [x] 4.1 Update `LocationResolverService` to inject `ComputedAtom<string | null>` for geodecodeApiKey instead of reading `mainGate.props` | ||
| - [x] 4.2 Update `MapsWidget.tsx` — derive `mapsToken` from the apiKey atom (or remove if LeafletMap/GoogleMap will read from atom directly) | ||
|
|
||
| ## 5. Tests | ||
|
|
||
| - [x] 5.1 Add unit test for `apiKeyAtom`: priority, fallback, null, and caching behavior | ||
| - [x] 5.2 Add unit test for `geodecodeApiKeyAtom`: same scenarios | ||
| - [x] 5.3 Update `LocationResolver` tests to inject atom mock instead of relying on gate props for apiKey | ||
| - [x] 5.4 Run full test suite and fix any failures |
60 changes: 60 additions & 0 deletions
60
...s-web/openspec/changes/archive/2026-06-17-simplify-maps-editor-config/design.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| ## Context | ||
|
|
||
| The Maps widget `getProperties()` function in `Maps.editorConfig.ts` contains branching logic for `platform === "desktop"` vs `"web"`. This separation no longer exists — Studio Pro uses a single editor. The `advanced` boolean property gates visibility of `mapProvider` and marker style options, adding unnecessary friction. The static `apiKey` string field should be deprecated in favor of the expression-based `apiKeyExp`. | ||
|
|
||
| Current `getProperties()` flow: | ||
|
|
||
| ``` | ||
| if (platform === "desktop") { | ||
| // show/hide apiKey vs apiKeyExp (static priority) | ||
| // hide "advanced" prop itself | ||
| } else { | ||
| // show/hide apiKey vs apiKeyExp (expression priority) | ||
| // gate mapProvider and marker styles behind "advanced" | ||
| } | ||
| ``` | ||
|
|
||
| ## Goals / Non-Goals | ||
|
|
||
| **Goals:** | ||
|
|
||
| - Single unified property visibility logic (no platform branching) | ||
| - Remove `advanced` property — all options always visible | ||
| - `apiKeyExp` always visible (never hidden) | ||
| - Deprecation warning when `apiKey` (static string) is used | ||
|
|
||
| **Non-Goals:** | ||
|
|
||
| - Removing `apiKey` from XML entirely (backward compatibility — existing apps use it) | ||
| - Changing runtime behavior (how the key is resolved at runtime stays the same) | ||
| - Touching `geodecodeApiKey` / `geodecodeApiKeyExp` show/hide logic beyond removing platform branching | ||
|
|
||
| ## Decisions | ||
|
|
||
| **1. Remove `advanced` from XML entirely** | ||
|
|
||
| The property serves no purpose once all options are always shown. Removing it from XML means Mendix will ignore any persisted value in existing apps — no migration needed. The widget typings will regenerate without it. | ||
|
|
||
| Alternative considered: Keep in XML but ignore it. Rejected — dead props confuse future developers. | ||
|
|
||
| **2. Unified apiKey/apiKeyExp visibility logic** | ||
|
|
||
| After removing platform branching, the logic becomes: | ||
|
|
||
| - `apiKeyExp` is always shown (never hidden) | ||
| - Hide `apiKey` if falsy, show otherwise | ||
|
|
||
| This preserves backward compat: users with only `apiKey` set still see their field, plus the new expression field. | ||
|
|
||
| **3. Deprecation via `check()` warning** | ||
|
|
||
| Add a `"warning"` severity problem in the `check()` function when `values.apiKey` is non-empty. Message directs users to use `apiKeyExp` instead. Using `check()` (not `getProperties()`) because that's where validation problems are surfaced in Studio Pro. | ||
|
|
||
| **4. Marker style visibility — always show** | ||
|
|
||
| Currently gated behind `!values.advanced` on web platform. After removing `advanced`, `markerStyle`/`customMarker` and `markerStyleDynamic`/`customMarkerDynamic` are always visible (conditional on `markerStyle === "image"` for the custom image field stays). | ||
|
|
||
| ## Risks / Trade-offs | ||
|
|
||
| - **[Breaking: `advanced` prop removed]** → Existing apps with `advanced: true` silently lose the property. No runtime impact — it was editor-only. Studio Pro handles missing props gracefully. | ||
| - **[Deprecation noise]** → Users with static `apiKey` see a new warning. This is intentional nudge, not an error. Using `"warning"` severity, not `"error"`. |
27 changes: 27 additions & 0 deletions
27
...web/openspec/changes/archive/2026-06-17-simplify-maps-editor-config/proposal.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| ## Why | ||
|
|
||
| The Maps widget editor config still has a web/desktop platform split that no longer exists in modern Studio Pro. This adds dead code paths and hides useful properties (like `mapProvider`) behind an "advanced" toggle that confuses users. Additionally, `apiKey` (static string) should be deprecated in favor of `apiKeyExp` (expression) for flexibility. | ||
|
|
||
| ## What Changes | ||
|
|
||
| - **BREAKING**: Remove the `advanced` boolean property from XML and editor config. Properties gated behind it (`mapProvider`, marker styles) become always visible. | ||
| - Remove the platform `"web"` / `"desktop"` conditional branching in `getProperties()`. All property visibility logic uses a single unified path. | ||
| - Stop hiding `apiKeyExp` — it is always shown as the primary API key field. | ||
| - Add a deprecation warning when the static `apiKey` property has a value, guiding users to use the `apiKeyExp` expression field instead. | ||
|
|
||
| ## Capabilities | ||
|
|
||
| ### New Capabilities | ||
|
|
||
| - `editor-config-simplified`: Unified property visibility logic without platform branching, removal of `advanced` toggle, and `apiKey` deprecation warning. | ||
|
|
||
| ### Modified Capabilities | ||
|
|
||
| _(none — no existing specs)_ | ||
|
|
||
| ## Impact | ||
|
|
||
| - `src/Maps.xml` — remove `advanced` property definition | ||
| - `src/Maps.editorConfig.ts` — rewrite `getProperties()` logic, add deprecation check to `check()` | ||
| - `typings/MapsProps.d.ts` — regenerated (loses `advanced` prop) | ||
| - Any container/config code referencing `props.advanced` (likely none beyond editor config) |
78 changes: 78 additions & 0 deletions
78
...e/2026-06-17-simplify-maps-editor-config/specs/editor-config-simplified/spec.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| ## ADDED Requirements | ||
|
|
||
| ### Requirement: No platform branching in property visibility | ||
|
|
||
| The `getProperties()` function SHALL NOT branch on the `platform` parameter. All property visibility logic MUST use a single unified code path. | ||
|
|
||
| #### Scenario: Same properties shown regardless of platform argument | ||
|
|
||
| - **WHEN** `getProperties()` is called with platform `"web"` or `"desktop"` | ||
| - **THEN** the returned properties are identical for both values | ||
|
|
||
| ### Requirement: Advanced property removed | ||
|
|
||
| The widget XML SHALL NOT define an `advanced` property. The editor config SHALL NOT reference `advanced` in any visibility logic. | ||
|
|
||
| #### Scenario: mapProvider always visible | ||
|
|
||
| - **WHEN** the widget is placed on a page | ||
| - **THEN** the `mapProvider` property is visible without any toggle | ||
|
|
||
| #### Scenario: Marker style options always visible | ||
|
|
||
| - **WHEN** a static or dynamic marker is configured | ||
| - **THEN** the `markerStyle` / `markerStyleDynamic` and `customMarker` / `customMarkerDynamic` properties are visible (custom marker still conditional on style being "image") | ||
|
|
||
| ### Requirement: apiKeyExp always visible | ||
|
|
||
| The `apiKeyExp` expression property SHALL never be hidden by `getProperties()`. | ||
|
|
||
| #### Scenario: Fresh widget shows expression field | ||
|
|
||
| - **WHEN** a new Maps widget is placed on a page with no configuration | ||
| - **THEN** `apiKeyExp` is visible to the user | ||
|
|
||
| #### Scenario: apiKeyExp visible even when apiKey has value | ||
|
|
||
| - **WHEN** `apiKey` (static) has a value set | ||
| - **THEN** `apiKeyExp` remains visible | ||
|
|
||
| ### Requirement: Static apiKey deprecation warning | ||
|
|
||
| The `check()` function SHALL return a warning-severity problem when `values.apiKey` is non-empty, informing the user that the static API key is deprecated and `apiKeyExp` (expression) should be used instead. | ||
|
|
||
| #### Scenario: Warning shown when static apiKey is set | ||
|
|
||
| - **WHEN** `values.apiKey` is a non-empty string | ||
| - **THEN** `check()` returns a problem with `severity: "warning"` on property `"apiKey"` with a message indicating deprecation | ||
|
|
||
| #### Scenario: No warning when apiKey is empty | ||
|
|
||
| - **WHEN** `values.apiKey` is empty or undefined | ||
| - **THEN** no deprecation warning is returned | ||
|
|
||
| ### Requirement: apiKey hidden when empty | ||
|
|
||
| The static `apiKey` field SHALL be hidden when it has no value. It SHALL only be shown when the user already has a value configured (for backward compatibility). | ||
|
|
||
| #### Scenario: apiKey hidden when empty | ||
|
|
||
| - **WHEN** `values.apiKey` is falsy (empty or undefined) | ||
| - **THEN** `apiKey` is hidden from the properties panel | ||
|
|
||
| #### Scenario: apiKey visible when it has a value | ||
|
|
||
| - **WHEN** `values.apiKey` is a non-empty string | ||
| - **THEN** `apiKey` is visible (for backward compatibility with existing configurations) | ||
|
|
||
| ## REMOVED Requirements | ||
|
|
||
| ### Requirement: Platform-specific property visibility | ||
|
|
||
| **Reason**: Web/desktop platform separation no longer exists in Studio Pro. | ||
| **Migration**: All properties use unified visibility logic. No user action needed. | ||
|
|
||
| ### Requirement: Advanced toggle for map options | ||
|
|
||
| **Reason**: Unnecessary UX friction. All options should be directly accessible. | ||
| **Migration**: Properties previously gated behind `advanced` are now always visible. Existing widgets with `advanced: true` will continue to work — the property is simply ignored. |
21 changes: 21 additions & 0 deletions
21
...ps-web/openspec/changes/archive/2026-06-17-simplify-maps-editor-config/tasks.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| ## 1. Remove `advanced` property | ||
|
|
||
| - [x] 1.1 Remove `advanced` property definition from `src/Maps.xml` | ||
| - [x] 1.2 Remove `advanced` from `mock-container-props.ts` | ||
|
|
||
| ## 2. Rewrite `getProperties()` in `src/Maps.editorConfig.ts` | ||
|
|
||
| - [x] 2.1 Remove the `platform` parameter and all platform branching (`if (platform === "desktop") / else`) | ||
| - [x] 2.2 Unify apiKey/apiKeyExp visibility: always show `apiKeyExp`, hide `apiKey` when it's falsy (only show if user has a value set) | ||
| - [x] 2.3 Remove all `advanced`-gated hiding logic (mapProvider, markerStyle, customMarker) | ||
| - [x] 2.4 Keep remaining conditional logic: Google-only props, OpenStreet hides apiKey, address/latLng toggle, customMarker conditional on style "image", geodecode keys hidden when no address markers | ||
|
|
||
| ## 3. Add deprecation warning | ||
|
|
||
| - [x] 3.1 In `check()`, add a warning-severity problem when `values.apiKey` is non-empty, message: "Static API key is deprecated. Use the 'API Key' expression instead." | ||
|
|
||
| ## 4. Cleanup and verify | ||
|
|
||
| - [x] 4.1 Regenerate typings (ensure `advanced` is gone from `MapsPreviewProps` and `MapsContainerProps`) | ||
| - [x] 4.2 Run lint and fix any issues | ||
| - [x] 4.3 Run tests and update snapshots if needed |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When expression is still loading, value is going to be
undefinedfor a bit bfore taking final shape, it shouldn't fall back toapiKeywhenapiKeyExpis not undefined. The sign that expression is not configured is that wholeapiKeyExpis undefined, not just value.