richardfogaca commented on PR #38374:
URL: https://github.com/apache/superset/pull/38374#issuecomment-4041343659
I think there’s still one logic gap in the viewport handling.
This fixes the “set a default viewport” path, but it does not fully fix the
control behavior when those fields are cleared again. In transformProps, empty
viewport values are normalized to undefined, but in MapBox.componentDidUpdate
the update logic only applies when the new prop is defined. That means if a
user sets lon/lat/zoom and later clears those controls back to blank, the map
keeps the last viewport state instead of falling back to fitBounds.
Concretely:
- transformProps converts '' to undefined
- componentDidUpdate ignores undefined viewport props
- result: clearing the controls has no effect
I’d expect blank viewport controls to restore the default fitBounds
behavior, not preserve the previous explicit viewport.
The current tests cover initial props, prop updates, and partial updates,
but I don’t see coverage for “set viewport, then clear it back to empty”. I’d
recommend adding that case and deciding whether clearing should recompute from
bounds or otherwise reset the internal viewport state.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
