deck.gl icon indicating copy to clipboard operation
deck.gl copied to clipboard

POC: PopupWidget

Open felixpalmer opened this issue 1 year ago • 5 comments

Background

Working on https://github.com/visgl/deck.gl/pull/8956 got me thinking whether it would be useful to add a PopupWidget. We would likely want to provide the same API eventually as in getTooltip, I've just implemented the textoption here

popup-widget

Change List

  • Add PopupWidget
  • Modify widget example to include popup

felixpalmer avatar Jun 18 '24 14:06 felixpalmer

Can we articulate the main advantages of PopupWidget compared to Deck's current tooltip prop?

@ibgreen getTooltip on works on hover, and we can only ever have a single popup. I note that in many of our examples we use alert() to demonstrate how to how to handle onClick but nobody is going to do this in practice and it forces our users to figure out how to handle this common scenario.

The elephant in the room is definitely the API misalignment with getTooltip. Choices:

  • A) Scrap the PopupWidget and instead add getPopup.
  • B) Remove getTooltip and document how to use it with PopupWidget (significant breaking change). In this POC it is sufficient to replace onClick with onHover and we have a tooltip!
  • C) Live with the difference, justified by the fact that getTooltip needs to render on hover and thus should be outside the full rendering loop

I'm leaning towards option B. Note that deck is already basically doing this and I've just realized that I've basically reimplemented Tooltip 👀

felixpalmer avatar Jun 19 '24 07:06 felixpalmer

Another point towards C & A is state management. In these, tooltip state is managed by deck for convenience and practicality. I'm pretty sure the python and declarative json environments rely on internally managed state, in particular.

I like the unification offered by B), so I think we'd just need a solution for python and weigh the tradeoff of making tooltips harder to get started with by removing the managed version.

chrisgervang avatar Jun 24 '24 15:06 chrisgervang

  • I also like option B. Can it be gradual (i.e. we deprecate getPopup()) or would we need to break apps?
  • Making sure widgets work in Python would be a strong acceptance criteria for me. The Python users can't easily create their own UIs on top of deck, so any widgets we can provide are extra valuable to this community.

ibgreen avatar Jun 24 '24 15:06 ibgreen

The widget manager has an imperative API... could this be made accessible to ~~python~~ all users not only for tooltips, but other stateful widgets as well? Is that all that Tooltip needs to be internally managed? It's currently added by default with this API already.

chrisgervang avatar Jun 24 '24 16:06 chrisgervang

A real "popup" widget will be expected to:

  • control placement relative to the anchor, or automatically positioned to fit in the map container;
  • have a tip that points to the anchor, like that from Google Maps or Maplibre;
  • can render multiple instances on map with the correct z order;
  • have a close button and/or closes when clicked outside;

The built-in tooltip in Deck is supposed to offer convenience but only the minimal functionality. While I'm fine with this simple initial implementation of a PopupWidget, it doesn't mean they are the same thing.

Pessimistress avatar Jun 24 '24 16:06 Pessimistress

Moved the popup update logic from the global scope into PopupWidget.props.onClick

ibgreen avatar Mar 02 '25 13:03 ibgreen

Coverage Status

coverage: 91.628% (+0.001%) from 91.627% when pulling e3cb29fc6f877f1298729c7accc91e98bd5e83e8 on felix/popup-widget into 16f5376af551364ad9fe37be78c832f9913e10b4 on master.

coveralls avatar Mar 02 '25 13:03 coveralls

Improved positioning:

  • Check boundaries of viewport and keep popup inside canvas.
  • Support a configurable minOffset from edge of canvas
  • Added a little "arrow" pointing to the selected "point"
  • Ensure position is recalculated while panning the view.

popup-position

ibgreen avatar Mar 28 '25 17:03 ibgreen

Renaming to _InfoWidget for now to avoid confusion with better PopupWidget initiatives.

ibgreen avatar Mar 28 '25 19:03 ibgreen

Thanks for pushing this one over the line @ibgreen :)

felixpalmer avatar Apr 03 '25 08:04 felixpalmer