Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(widgets) Custom Widget Developer Guide #9304

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

chrisgervang
Copy link
Collaborator

For #7946

Background

Adding developer guide to include more advanced examples about how to write both react and non-react widgets. This way our getting-started examples can be very simple, and we can have something to point more advanced users to when they're writing their own widgets.

Change List

  • docs

@chrisgervang chrisgervang changed the title docs(widgets) Widget Developer Guide docs(widgets) Custom Widget Developer Guide Dec 18, 2024
@chrisgervang chrisgervang requested a review from ibgreen December 18, 2024 05:25
@coveralls
Copy link

coveralls commented Dec 18, 2024

Coverage Status

coverage: 91.701% (-0.006%) from 91.707%
when pulling 98a82a4 on chr/widget-dev-guide
into 05bca74 on master.

Copy link
Collaborator

@felixpalmer felixpalmer left a comment

Choose a reason for hiding this comment

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

Great job ❤️

docs/developer-guide/custom-widgets/README.md Outdated Show resolved Hide resolved
{layers.filter(layer => (
this.deck?.props.layerFilter({layer, viewport})
)).map((layer) => {
<li key={layer.id}>{layer.id}</li>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be much more code to show how control the layers in some way? For example, have a checkbox next to each layer to toggle visibility in that view?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can try it.. but controlling anything the user sets starts to get into a territory where user state and internal state can get out of sync. At least, there's nothing preventing user-space from just overriding any changes a widget makes to a layer. And there's some risk of introducing a race condition.

I think what this would look like with our current API is something like:

this.deck?.setProps({ layers: layers.map(layer => layer.clone({ visible: isVisible }) })

This could cause events to fire in the app that in-turn call setProps. Is there a way to draw the layers without alerting the user? Perhaps:

this.deck?._drawLayers('layer-list-widget', { layers: ... })

Copy link
Collaborator

@ibgreen ibgreen left a comment

Choose a reason for hiding this comment

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

This is awesome work.

FWIW - One thing that I suspect will make widgets critical for me over the next year is the ability to use them in Python/pydeck. It could be a next step, making these available in python.

docs/developer-guide/custom-widgets/README.md Outdated Show resolved Hide resolved
```ts
import type {WidgetPlacement} from '@deck.gl/core'

interface AwesomeWidgetProps {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: Recommend with the arguably more modern approach to use the type keyword here. I personally prefer reserving interface for types that are to be derived from with methods that need to be implemented.

Suggested change
interface AwesomeWidgetProps {
export type AwesomeWidgetProps = {

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'd want to do this in a separate PR and update everything in one sweep.

/**
* Widget positioning within the view. Default: 'top-left'.
*/
placement?: WidgetPlacement;
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider exporting a set of base WidgetProps so that widget writers don't need to retype all of that?

Suggested change
placement?: WidgetPlacement;
export type WidgetProps = {
id?: string;
placement?: WidgetPlacement;
viewId?: string | null;
};
...
export type AwesomeWidgetProps = WidgetProps & {
customText: string;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I like the idea to encourage consistency, though it's still 100% up to the widget authors to decide how they implement this since we're only defining an interface.

docs/developer-guide/custom-widgets/README.md Outdated Show resolved Hide resolved
}
```

## Best Practices
Copy link
Collaborator

Choose a reason for hiding this comment

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

This section looks a little "lost" here at the very end of the page. Maybe lead with something like this before all the examples?. Or maybe the section will grow and it will look more natural.

docs/developer-guide/custom-widgets/universal-widgets.md Outdated Show resolved Hide resolved

## Handling View Interaction Events

A widget can update in response to a user interacting with the deck.gl view the widget is attached to.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is easily the most important section and needs its own page IMHO. (I do understand that we have work to do here.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Say more? What kind of example would you like to see?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I've been struggling to port the graph layers controller React component to a widget, I was looking for docs on how to implement zooms and pans

https://github.com/visgl/deck.gl-community/blob/master/modules/graph-layers/src/widgets/view-control-widget.tsx#L325

docs/developer-guide/custom-widgets/universal-widgets.md Outdated Show resolved Hide resolved

Below is a comprehensive example demonstrating a layer list widget implemented using Preact for dynamic UI rendering:

```tsx
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should really use MDX capabilities and make these widgets live on the pages...

}
```

##### Applying the deck.gl widget design system
Copy link
Collaborator Author

@chrisgervang chrisgervang Dec 18, 2024

Choose a reason for hiding this comment

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

Should applying the design system be it's own page?

@chrisgervang chrisgervang mentioned this pull request Dec 19, 2024
36 tasks
// Handle when props change here.
this.placement = props.placement ?? this.placement;
this.viewId = props.viewId ?? this.viewId;
this.props = {...props};
Copy link
Collaborator

Choose a reason for hiding this comment

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

This could wipe out existing props?

Suggested change
this.props = {...props};
this.props = {...this.props, ...props};

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.

4 participants