Skip to content

Fix broken styling of map attributions - #7526

Merged
emilykl merged 2 commits into
masterfrom
map-error-layout-issue-2
Aug 27, 2025
Merged

Fix broken styling of map attributions #7526
emilykl merged 2 commits into
masterfrom
map-error-layout-issue-2

Conversation

@emilykl

Copy link
Copy Markdown
Contributor

Closeshttps://github.com/plotly/plotly-studio/issues/1175

Map attributions were styled incorrectly for plots using MapLibre (scattermap, densitymap, scattermap traces).

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.js file (not included in this PR since we only update dist/ during the release process).


As an aside, this was not caught by the image tests because the Plotly.toImage() function for MapLibre plots calls map.getCanvas() which does not include the attribution, so Plotly.js manually adds the attribution to the generated image.

@emilyklemilykl self-assigned this Aug 18, 2025
@camdecoster

Copy link
Copy Markdown
Contributor

As we discussed, with this change, I think that this section can be removed:

if(pathToBundle.endsWith('.js')){
varlen=pathToBundle.length;
varcssOutput=pathToBundle.slice(0,len-3)+'.css';
// remove unwanted css file
if(fs.existsSync(cssOutput)){
fs.unlinkSync(cssOutput);
}
}

@emilykl
emilyklforce-pushed the map-error-layout-issue-2 branch from 9ee114c to 70e96f8CompareAugust 20, 2025 21:04
@emilykl

Copy link
Copy Markdown
ContributorAuthor

@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.

@gvwilsongvwilson added P1 needed for current cycle fix fixes something broken labels Aug 22, 2025
@camdecoster

Copy link
Copy Markdown
Contributor

@emilykl

Copy link
Copy Markdown
ContributorAuthor

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.css file (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.

cc @gvwilson@camdecoster

@emilykl
emilyklforce-pushed the map-error-layout-issue-2 branch from 70e96f8 to c8af613CompareAugust 27, 2025 18:24

@camdecostercamdecoster 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.

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?

@emilykl

Copy link
Copy Markdown
ContributorAuthor

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.css file (see recent comments on that PR). We should revisit and fix when we have the bandwidth.

#7543

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.

#7544

@emilykl
emilykl merged commit 3a8df23 into masterAug 27, 2025
6 checks passed
@emilykl
emilykl deleted the map-error-layout-issue-2 branch August 27, 2025 20:24
@gvwilson

Copy link
Copy Markdown
Contributor

yay!

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

Labels

fixfixes something brokenP1needed for current cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@emilykl@camdecoster@gvwilson