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

Add Plot status events to vgplot #157

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spren9er
Copy link
Contributor

Why is this pull request necessary?

When using vgplot, there is currently no way to keep track of the status of a single plot (e.g. is plot refreshing or already done?).
Statuses could be helpful for showing a spinner, while data is updating (see example below).
In a component framework, it could make sense to work with a Plot class directly (one component renders a single plot). Also, in that way an already existing element (SVGSVGElement or HTMLDivElement) can be used, instead of a newly generated one of vg.plot.

What does this pull request cover?

This pull request adds more convenience methods for building and working with a Plot instance.

  • Add following attributes/methods to Plot class
    • status: attribute (one of idle, pendingQuery or pendingRender)
    • connect: method for connecting all Plot marks to coordinator
    • addDirectives: method for adding directives to Plot
    • addEventListener: method for adding event listener for status value
    • removeEventListener: method for removing event listener for status value
  • Add parsePlotSpec to parse-spec.js for creating a Plot instance from JSON specification
  • Add all above attributes/methods to documentation

How to create a Plot instance?

Create a Plot instance directly from class

import { Plot } from '@uwdata/vgplot';

const plot = new Plot(element);

plot.addDirectives(...);

Create a Plot instance from JSON specification via parsePlotSpec

import { parsePlotSpec, type Spec, type Param } from '@uwdata/vgplot';

const spec: Spec = { plot: [...], width: 800, height: 600 };
const params: [string, Param][] = [...];

const plot = await parsePlotSpec(spec, { params }, element);

Then, on an existing Plot instance, one can add an event listener for status updates.
Finally, all marks need to be connected to Mosaic coordinator with connect method.

plot.addEventListener('status', (status: string) => {
  // do something with `status`
});

await plot.connect();

Breaking changes

None, as existing API has not changed.

Example: Rendering a dashboard with several Mosaic plots

When using several single vgplot plots in a dashboard, it is sometimes unclear which plots are still updating and which have already been re-rendered. A small example (more plots → more confusion):

mosaic-plot-observer-before-small.mov

Using status events of Plot, we can indicate that a plot is outdated (here via spinner and transparency).

mosaic-plot-observer-after-small.mov

Note about this pull request

This pull request was created, because of the specific need of embedding vgplot plots in a pure SVG dashboard application. Feel free to do whatever you like with it.
If Plot classes should not be used directly in above way or it is against vgplot philosophy, just ignore it.

@jheer
Copy link
Member

jheer commented Sep 21, 2023

Thanks, I certainly see the value of adding async status updates! I do wonder if we can adopt an approach that is consistent across Mosaic applications. It is a bit complicated because in some cases an interface component might consist of a single client while in other cases (as with a Plot with multiple marks) we're actually dealing with a group of clients.

Perhaps we can add a stand-alone status handler as an API library element, and extend Plot to use that? Then other single- or multi-client components could use the same infrastructure for consistency. Happy to consider any thoughts on how to approach this is an elegant way.

Meanwhile, this PR includes some other aspects that we might want to remove or handle separately, such as the changes involving directives and plot spec parsing.

@domoritz
Copy link
Member

@spren9er do you want to revise this pull request?

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