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

Allow users to set preserveDrawingBuffer in the options #199

Merged
merged 1 commit into from
Oct 6, 2023

Conversation

sheeley820
Copy link
Contributor

It was discovered that printing maps rendered with esri-leaflet-vector does not work in Firefox.

An issue was created here.

This PR fixes that issue by allowing the user to set preserveDrawingBuffer to true, which resolves the issue. It is set to false by default in the maplibre-gl-js project in the scr/ui/map.ts file for performance optimizations. Adding the ability for a user to toggle this value through esri-leaflet-vector, but maintaining a default of false will help users who want to provide printing capabilities, while maintaining the performance optimization for those who do not have this need.

@gavinr-maps
Copy link
Contributor

Thank you for the PR.

If we Expose the MaplibreGLJSLayer Variable (#197), this PR #199 is not needed, correct? Because you could modify the preserveDrawingBuffer property on the exposed MaplibreGLJSLayer Variable.

@sheeley820
Copy link
Contributor Author

sheeley820 commented Sep 28, 2023

@gavinr-maps So technically it is possible to do the following even without MaplibreGLJSLayer being exposed:

const vector = L.esri.Vector.vectorBasemapLayer(basemapEnum, {
        apiKey: apiKey
    })
vector.options.preserveDrawingBuffer = true
vector.addTo(map)

And this resolves the printing error, but for me it seems like since we already allow MapLibre options to be passed directly into the VectorTileLayer initialization (pane, opacity) it would make sense to allow perserveDrawingBuffer to be passed in through the VectorBasemapLayer and VectorTileLayer constructors as well.

Copy link
Contributor

@patrickarlt patrickarlt left a comment

Choose a reason for hiding this comment

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

This looks good to me.

@gavinr-maps
Copy link
Contributor

Looks good. In the future if people need access to more options, we should introduce a mapLibreMapOptions property. But we can avoid prematurely optimizing by just going with this for now.

@gavinr-maps gavinr-maps merged commit 5e6aa8c into Esri:master Oct 6, 2023
9 checks passed
@gavinr-maps
Copy link
Contributor

This was released in v4.2.0.

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.

3 participants