Skip to content

Map: adapt OSM marker colors to Fluent Next - #35768

Open
AlisherAmonulloev wants to merge 2 commits into
feature/26_2_osm-provider-for-dxmap/mainfrom
26_2_osm_fluent_next
Open

AlisherAmonulloev wants to merge 2 commits into
feature/26_2_osm-provider-for-dxmap/mainfrom
26_2_osm_fluent_next

Conversation

@AlisherAmonulloev

Copy link
Copy Markdown
Contributor

No description provided.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

Static review found consistent styling, token publication, and regression coverage with no blocking issues; tests were not executed.

No open issues 🎉

What changed in this PR

Adapts OpenStreetMap markers to Fluent Next colors through public component custom properties.

Changes:

  • Adds marker background and content tokens and applies them to marker shapes.
  • Updates naming and scope metadata, with regression tests across both sizes and color modes.
File Description
packages/​devextreme-scss/​tools/​review/​nested-scopes.json Registers marker child scopes.
packages/​devextreme-scss/​tools/​naming/​registries.json Adds the marker part.
packages/​devextreme-scss/​tools/​naming/​derive-registries.ts Defines the marker registry override.
packages/​devextreme-scss/​tests/​public-surface.baseline.json Records new public properties.
packages/​devextreme-scss/​tests/​map-marker.test.ts Tests marker styling and token mappings.
packages/​devextreme-scss/​scss/​widgets/​fluent-next/​map/​_public.scss Publishes marker color properties.
packages/​devextreme-scss/​scss/​widgets/​fluent-next/​map/​_index.scss Applies marker fill and stroke colors.
packages/​devextreme-scss/​scss/​widgets/​fluent-next/​map/​_colors.scss Defines marker color defaults.

Review effort: Balanced

@EugeniyKiyashko EugeniyKiyashko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The SCSS part looks right: the bg/content pair matches badge and contained button, the OpenLayers marker lives inside .dx-map-container so the published names reach .dx-map-marker-default-*, and both modes resolve to primary-100 / neutral-10 (checked on a local token build). All my comments are about tests/map-marker.test.ts.

Overall I would drop the file: naming:check in styles.yml already proves both halves of the first test (publish.ts --check for the _public.scss lines, reachability.ts for the var() reads landing inside the map root), and the second test pins the token package rather than the map. If you want to keep a regression test for the fill/stroke/center pairing, one test.each(['light', 'dark']) with the three asserts from lines 21-26, plus jest.setTimeout, is enough.

Comment thread packages/devextreme-scss/tests/map-marker.test.ts Outdated
Comment thread packages/devextreme-scss/tests/map-marker.test.ts Outdated
Comment thread packages/devextreme-scss/tests/map-marker.test.ts Outdated
Comment thread packages/devextreme-scss/tests/map-marker.test.ts Outdated

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants