Skip to content

Conversation

ibgreen
Copy link
Collaborator

@ibgreen ibgreen commented May 4, 2025

Closes #

Background

Applications that use predefined widgets may want an easy way to add some custom buttons that become part of the widget layout (positioning, theming).

  • A widget that lets the user add custom icon buttons to deck
  • The buttons participate in widget positioning and theming,
  • however the functionality is defined by the props.onButtonClick callback

TODO

  • We could create a custom MenuWidget / IconMenuWidget as well
  • ZoomWidget could inherit from ButtonGroupWidget
  • SplitViewWidget and maybe ContextMenuWidget could inherit from IconMenuWidget
  • ...

Change List

  • one notable thing is that we allow icons to be defined either through class-names or by providing SVG.
  • Though the latter exposes preact in the interface which we may not want, a better/alternative strategy may be desired.

@ibgreen ibgreen requested a review from chrisgervang May 4, 2025 23:01
@coveralls
Copy link

Coverage Status

coverage: 91.634% (+0.001%) from 91.633%
when pulling 4d84f0f on ib/custom-widgets
into 53cb332 on master.

@chrisgervang
Copy link
Collaborator

chrisgervang commented May 4, 2025

At a high level, I can see the utility of making it easy for users to define custom controls via buttons and menus, but I'm not sure how far we want to take this idea.

Curious if you have thoughts on this @felixpalmer and @Pessimistress?

Though the latter exposes preact in the interface which we may not want, a better/alternative strategy may be desired.

We need to keep preact out of the API interface. A user can instead define a custom icon as a css class following the documented steps we wrote for this.

Also, accepting a native SvgElement was originally considered but we decided to define them in CSS and use CSS masking to color them. I don't think we should change this pattern for a utility widget.. it's a small effort to add an icon to your own stylesheet.

Maybe we come up with a better way to defining icons one day, but for now I only want to maintain one way of doing it.
No

@ibgreen
Copy link
Collaborator Author

ibgreen commented May 5, 2025

If there is too much uncertainty some widgets could go into deck.gl-community.

I was initially considering developing all the new widgets there, so that deck could cherry pick among them, but hard to do until we publish the new Widget class.

I do respect your stance on icons especially for built in icons but personally I don't like having to mess with separate CSS in my app code.

Ageee that preact shouldn't be exposed and svg may not have a natural non-preact interface, but I feel that users may want to use normal PNG images for icons, i.e it could make a lot of sense to accept an ImageBitmap for this type of custom widgets.

@chrisgervang
Copy link
Collaborator

I've come around on this a bit as I thought through some use cases.

I think there's value in offering a couple basic UI controls, prioritizing the ones our core widgets use themselves. A button, button group, drop down select menu, and text input submit component all seem like good candidates. Radix Primitives are a good reference for API inspiration - the scariest part about this offering to me is the potential for bike-shedding the widget names and props. I'd rather avoid the problem by borrowing from an existing system.

As an aside, I don't think python users would be able to get utility from this unless they had some mechanism for bi-directional syncing... @bijanvakili is bi-directional sync support something your team has prioritized by any chance? I was looking for an issue tracker about pydeck goals and couldn't find one

/** Icons can be loaded from style sheet using class name */
className: string;
/** Alterhnatively an SVG icon element can be provided */
icon?: () => JSX.Element;
Copy link
Collaborator

@chrisgervang chrisgervang Sep 26, 2025

Choose a reason for hiding this comment

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

I learned today that this should should just be a JSX.Element.

Suggested change
icon?: () => JSX.Element;
icon?: JSX.Element;

The reason the function seemed to be needed was that defining JSX on the module level gets immediately executed by node tests as React code instead of Preact code due to the way our tests are configured. We could fix the configuration.. but in the meantime we should just type our preact code correctly and wrap all JSX in a function.

See 62fe293

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants