Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions draftlogs/7884_fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix `scattermap`, `densitymap` traces not showing all points by dynamically computing `center`, `zoom` values [[#7884](https://github.com/plotly/plotly.js/pull/7884)], with thanks to @palmerusaf and @DhruvGarg111 for the contributions!
10 changes: 4 additions & 6 deletions src/plots/map/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,17 +70,15 @@ var styleValuesMap = sortObjectKeys(stylesMap);

module.exports = {
styleValueDflt: 'basic',
stylesMap: stylesMap,
styleValuesMap: styleValuesMap,

stylesMap,
styleValuesMap,
traceLayerPrefix: 'plotly-trace-layer-',
layoutLayerPrefix: 'plotly-layout-layer-',

missingStyleErrorMsg: [
'No valid maplibre style found, please set `map.style` to one of:',
styleValuesMap.join(', '),
'or use a tile service.'
].join('\n'),

mapOnErrorMsg: 'Map error.'
mapOnErrorMsg: 'Map error.',
fitBoundsPadding: 20
};
76 changes: 76 additions & 0 deletions src/plots/map/get_map_fit_bounds.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
import { getFitboundsLonRange } from '../../lib/geo_location_utils';
import type { MapLayout, ScattermapData } from '../../types/generated/schema';

// Same shape as the user-facing `map.bounds` attribute, but with all fields required
type LonLatBox = Required<NonNullable<MapLayout['bounds']>>;

// Minimal shape of the fullData entries this helper reads
interface FitBoundsTrace extends Pick<ScattermapData, 'subplot' | 'visible'> {
// Tighten lat/lon to be more specific than default
lat?: ArrayLike<number>;
lon?: ArrayLike<number>;
// Broaden type since this could run against multiple trace types
type?: string;
}

/**
* Compute a lon/lat bounding box from lonlat-bearing traces (`scattermap`,
* `densitymap`) on the given map subplot.
*
* Returns null when:
* - no fittable data exists on the subplot;
* - a location-based trace (`choroplethmap`) is present — those carry
* `locations`/`geojson`, not raw lon/lat, and need geojson bbox handling
* that isn't implemented here.
*
* @param fullData - The full data array (post supply-defaults)
* @param subplotId - e.g. `'map'`, `'map2'`
*/
export function getMapFitBounds(fullData: FitBoundsTrace[], subplotId: string): LonLatBox | null {
const validLons: number[] = [];
let minLon = Infinity;
let maxLon = -Infinity;
let minLat = Infinity;
let maxLat = -Infinity;

for (const trace of fullData) {
if (trace.subplot !== subplotId || trace.visible !== true) continue;

// choroplethmap traces carry locations/geojson, not raw lon/lat; bail
// out rather than frame around a subset of the subplot's data.
if (trace.type === 'choroplethmap') return null;

const { lat, lon } = trace;
if (!lon || !lat) continue;

const len = Math.min(lon.length, lat.length);
for (let j = 0; j < len; j++) {
const lo = lon[j];
const la = lat[j];
if (Number.isFinite(lo) && Number.isFinite(la)) {
validLons.push(lo);
// Track longitude bounds
if (lo < minLon) minLon = lo;
if (lo > maxLon) maxLon = lo;
// Track latitude bounds
if (la < minLat) minLat = la;
if (la > maxLat) maxLat = la;
}
}
}

if (!validLons.length) return null;

let west = minLon;
let east = maxLon;
// Only handle antimeridian if it actually gets crossed
if (maxLon - minLon > 180) {
const lonRange = getFitboundsLonRange(validLons);
if (lonRange) {
west = lonRange[0];
east = lonRange[1];
}
}

return { west, east, south: minLat, north: maxLat };
}
9 changes: 0 additions & 9 deletions src/plots/map/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,6 @@ exports.plot = function plot(gd) {
fullLayout[id]._subplot = map;
}

if(!map.viewInitial) {
map.viewInitial = {
center: Lib.extendFlat({}, opts.center),
zoom: opts.zoom,
bearing: opts.bearing,
pitch: opts.pitch
};
}

map.plot(subplotCalcData, fullLayout, gd._promises);
}
};
Expand Down
42 changes: 22 additions & 20 deletions src/plots/map/layout_defaults.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,19 @@ var Lib = require('../../lib');
var handleSubplotDefaults = require('../subplot_defaults');
var handleArrayContainerDefaults = require('../array_container_defaults');
var layoutAttributes = require('./layout_attributes');

const { getMapFitBounds } = require('./get_map_fit_bounds');

module.exports = function supplyLayoutDefaults(layoutIn, layoutOut, fullData) {
handleSubplotDefaults(layoutIn, layoutOut, fullData, {
type: 'map',
attributes: layoutAttributes,
handleDefaults: handleDefaults,
partition: 'y'
handleDefaults,
partition: 'y',
fullData
Comment thread
camdecoster marked this conversation as resolved.
});
};

function handleDefaults(containerIn, containerOut, coerce) {
function handleDefaults(containerIn, containerOut, coerce, opts) {
coerce('style');
coerce('center.lon');
coerce('center.lat');
Expand All @@ -28,12 +29,7 @@ function handleDefaults(containerIn, containerOut, coerce) {
var east = coerce('bounds.east');
var south = coerce('bounds.south');
var north = coerce('bounds.north');
if(
west === undefined ||
east === undefined ||
south === undefined ||
north === undefined
) {
if (west === undefined || east === undefined || south === undefined || north === undefined) {
delete containerOut.bounds;
}

Expand All @@ -42,6 +38,12 @@ function handleDefaults(containerIn, containerOut, coerce) {
handleItemDefaults: handleLayerDefaults
});

// Auto-frame the initial view to the data
if (containerIn.center === undefined && containerIn.zoom === undefined) {

@emilykl emilykl Jul 7, 2026

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.

Should skip this step if bounds is defined.

Also, I'm not positive, but I think we should test containerOut rather than containerIn, so that if the user supplies an invalid center or zoom value, we ignore it and compute the auto-bounds anyway.

Edit: Never mind, I misunderstood the bounds param. I do think we should test on containerOut though.

Suggested change
if (containerIn.center === undefined && containerIn.zoom === undefined) {
if (containerOut.center === undefined && containerOut.zoom === undefined) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If we test containerOut, center/zoom will always be set because coerce supplies default values for those attributes. If containerIn doesn't have center/zoom, that's a signal that we need to provide bounds for the map. If the user supplies junk, this will still work because the call to getMapFitBounds won't run and coerce already took care of the junk values.

@emilykl emilykl Jul 7, 2026

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.

Oh yeah of course we can't use containerOut, that makes sense.

But that does mean auto-bounds won't be applied if the user supplies junk values for center and zoom, right?

(That seems like the wrong behavior to me, but I could be convinced otherwise.)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm deferring to the user in this case. If they explicitly set values that would otherwise be set by auto-fitting, I opted to skip auto-fitting. If you want to provide garbage values, that's up to you. We could update that behavior in the future, but I think we should trust the user for now.

const fitBounds = getMapFitBounds(opts.fullData, opts.id);
if (fitBounds) containerOut._fitBounds = fitBounds;
}

// copy ref to input container to update 'center' and 'zoom' on map move
containerOut._input = containerIn;
}
Expand All @@ -52,27 +54,27 @@ function handleLayerDefaults(layerIn, layerOut) {
}

var visible = coerce('visible');
if(visible) {
if (visible) {
var sourceType = coerce('sourcetype');
var mustBeRasterLayer = sourceType === 'raster' || sourceType === 'image';

coerce('source');
coerce('sourceattribution');

if(sourceType === 'vector') {
if (sourceType === 'vector') {
coerce('sourcelayer');
}

if(sourceType === 'image') {
if (sourceType === 'image') {
coerce('coordinates');
}

var typeDflt;
if(mustBeRasterLayer) typeDflt = 'raster';
if (mustBeRasterLayer) typeDflt = 'raster';

var type = coerce('type', typeDflt);

if(mustBeRasterLayer && type !== 'raster') {
if (mustBeRasterLayer && type !== 'raster') {
type = layerOut.type = 'raster';
Lib.log('Source types *raster* and *image* must drawn *raster* layer type.');
}
Expand All @@ -83,20 +85,20 @@ function handleLayerDefaults(layerIn, layerOut) {
coerce('minzoom');
coerce('maxzoom');

if(type === 'circle') {
if (type === 'circle') {
coerce('circle.radius');
}

if(type === 'line') {
if (type === 'line') {
coerce('line.width');
coerce('line.dash');
}

if(type === 'fill') {
if (type === 'fill') {
coerce('fill.outlinecolor');
}

if(type === 'symbol') {
if (type === 'symbol') {
coerce('symbol.icon');
coerce('symbol.iconsize');

Expand All @@ -105,7 +107,7 @@ function handleLayerDefaults(layerIn, layerOut) {
noFontVariant: true,
noFontShadow: true,
noFontLineposition: true,
noFontTextcase: true,
noFontTextcase: true
});
coerce('symbol.textposition');
coerce('symbol.placement');
Expand Down
Loading