-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix broken styling of map attributions #7526
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
Conversation
As we discussed, with this change, I think that this section can be removed:
plotly.js/tasks/util/bundle_wrapper.mjs
Lines 49 to 57 in 04441f9
9ee114c to
70e96f8
Compare
@birkskyum What's your take on the best way to include the MapLibre stylesheet in the Plotly.js bundle? It turns out the CSS was never being correctly included following the switch to ESbuild, since we distribute only a single .js file for Plotly.js, but the CSS was being extracted into separate files which were simply ignored.
For future reference if we want to understand the history this issue:
Some notes on this change:
-
This fix will likely not work in strict CSP environments which disallow inline CSS. However, it also doesn't make the status quo worse except for adding additional console noise. Those unable to use inline CSS may include the stylesheet https://unpkg.com/maplibre-gl@4.7.1/dist/maplibre-gl.css in a way permitted by their CSP.
-
It seems that some of the work done in Remove inline styles that break plots in strict CSP setups #7109 to make Plotly.js work better in strict CSP environments was undone by the migration to ESBuild. For example, we no longer produce a separate
plotly.cssfile (see recent comments on that PR). We should revisit and fix when we have the bandwidth. -
This approach adds about 45 kB to the Plotly.js bundle size; ideally we should consider including only the styles which are strictly necessary for rendering Maplibre charts correctly in Plotly.js (which seems to be about 4 kB for the attributions). But, that adds additional complexity to the build process as we'd need to include a script to parse and subset the MapLibre CSS, so I'm choosing not to do it for now.
70e96f8 to
c8af613
Compare
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.
Looks good. This will fix things for all but those that disallow inline CSS, and I don't think that's a big number of users. Could you please create an issue to follow up on this in the future and either trim the CSS or address the inline CSS?
Issues opened:
It seems that some of the work done in #7109 to make Plotly.js work better in strict CSP environments was undone by the migration to ESBuild. For example, we no longer produce a separate
plotly.cssfile (see recent comments on that PR). We should revisit and fix when we have the bandwidth.
This approach adds about 45 kB to the Plotly.js bundle size; ideally we should consider including only the styles which are strictly necessary for rendering Maplibre charts correctly in Plotly.js (which seems to be about 4 kB for the attributions). But, that adds additional complexity to the build process as we'd need to include a script to parse and subset the MapLibre CSS, so I'm choosing not to do it for now.
yay!
Closes https://github.com/plotly/plotly-studio/issues/1175
Map attributions were styled incorrectly for plots using MapLibre (
scattermap,densitymap,scattermaptraces).This was because a MapLibre stylesheet was missing due to being incorrectly handled in the bundling process.
The bug was introduced by the migration to esbuild (#6909) and present as of Plotly.js 3.0.0.
This PR fixes that issue by using a different tool to inline the missing CSS directly into the JS bundle.
Before fix:
Screenshot 2025年08月18日 at 12 39 25 PM
After fix:
Screenshot 2025年08月18日 at 12 39 09 PM
This commit demonstrates the resulting difference in the
dist/plotly.jsfile (not included in this PR since we only updatedist/during the release process).As an aside, this was not caught by the image tests because the
Plotly.toImage()function for MapLibre plots callsmap.getCanvas()which does not include the attribution, so Plotly.js manually adds the attribution to the generated image.