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

feat: allow merging StyleImport configs #3619

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bvanderdrift
Copy link
Contributor

Description

Added optional merge property to StyleImport that allows merging an import config with the existing style import config, instead of full override.

Defaults to false when not defined, to stay in line with current behaviour of StyleImport.

I haven't tested this on the example app and/or added an example, since I'm on some strict deadlines. Would love to get some help getting this PR over the finish line.

Checklist

  • I've read CONTRIBUTING.md
  • I updated the doc/other generated code with running yarn generate in the root folder
  • I have tested the new feature on /example app.
    • In V11 mode/ios
    • In New Architecture mode/ios
    • In V11 mode/android
    • In New Architecture mode/android
  • I added/updated a sample - if a new feature was implemented (/example)

Screenshot OR Video

Tested this both on iOS & Android, but for simplicity I only added iOS demo video's here.

Test explained: the map style has both POI not showing & light mode dusk.

merge unset

Expected: full override, so 'POI showing' falls back to default (true).

Screen.Recording.2024-09-11.at.10.24.53.mov

merge: false

Expected: full override, so 'POI showing' falls back to default (true).

Screen.Recording.2024-09-11.at.10.26.52.mov

merge: true

Expected: merging style, so 'POI showing' stays as defined in the style (false).

Screen.Recording.2024-09-11.at.10.27.26.mov

Component to test with

import Mapbox, {StyleImport} from '@rnmapbox/maps';
import React from 'react';

const DemoMap = () => {
  return (
    <Mapbox.MapView
      style={{flex: 1}}
      styleURL="mapbox://styles/beerd/cm0xlddce00zx01pm5zyb9om9">
      <Mapbox.Camera
        defaultSettings={{
          centerCoordinate: [4.9041, 52.3676],
          zoomLevel: 13,
        }}
        centerCoordinate={[4.9041, 52.3676]}
      />
      <StyleImport
        id="basemap"
        existing
        config={{
          lightPreset: 'night',
        }}
        merge={true}
      />
    </Mapbox.MapView>
  );
};

export default DemoMap;

@mfazekas
Copy link
Contributor

@bvanderdrift thanks much looks amazing. 👍

Copy link
Contributor

@mfazekas mfazekas left a comment

Choose a reason for hiding this comment

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

Looks good, but we'd need to add to new architecture src/specs/RNMBXStyleImportNativeComponent.ts and co.

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.

2 participants