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

[BUG] Vislib to Vega-lite migrator, fix chart overflow issue #3485

Closed
Tracked by #2880
lezzago opened this issue Feb 22, 2023 · 5 comments
Closed
Tracked by #2880

[BUG] Vislib to Vega-lite migrator, fix chart overflow issue #3485

lezzago opened this issue Feb 22, 2023 · 5 comments
Assignees
Labels
bug Something isn't working feature-anywhere unified visualization UX visualizations Issues and PRs related to visualizations

Comments

@lezzago
Copy link
Member

lezzago commented Feb 22, 2023

Describe the bug

For charts that are being rendered with vega-lite instead of vislib, the chart has an overflow error where you need to scroll in the chart to view the entire chart as there is some css styling problem that doesnt size the chart correctly when displaying on a visualize embeddable. This is related to the #2880 feature and came from the #3106 PR.

To Reproduce
Create a line chart that is rendered by vega-lite

Expected behavior
Have the chart fix in the embeddable object without the need to scroll.

@ohltyler
Copy link
Member

It looks that autosize isn't working correctly. When calling autosize() on the view itself, we can update to have it show correctly.

For example, adding below line in updateVegaSize() will resolve the problem.

       view.autosize({
          type: 'fit',
          contains: 'padding',
        })

Still needs to be researched more since there's no documentation on this

@ohltyler
Copy link
Member

ohltyler commented Apr 13, 2023

Adding things here as I find them:

  • the vega lite spec (set to vlspec in vegaparser) is compiled in vega_fn (the vega expr fn)
  • even with a set autosize value in the vega spec, in the context of vconcat, this is getting ignored. For example, setting the spec autosize field to { type: 'fit', contains: 'padding'} is ignored, and the default value of {type: 'pad', contains: 'padding'} is set in the vega spec (the compiled spec generated from the vega lite spec that is controlled by us). From documentation, it makes sense that this isn't supported. But, even when setting the autosize values on the individual views within the spec to our desired value, those seem to be getting ignored as well
  • there is a autosize signal provided in the vega view (unable to find docs on this method being used or existing). when updating that with our desired autosize values of {type: 'fit', contains: 'padding'}, this updates our view correctly, and fixes the scrolling issues, and resizing the window etc. resizes the chart as expected with no overflow
  • in vega_view (the implemented default view extending vega_base_view), there is an _initViewCustomizations() fn. Within here, the view width/height values are dynamically set based on the container the view is in. See updateVegaSize() for details.

From the documentation:

  • "pad" will "often exceed the specified width, height, and padding" to make all content visible (the default)
  • "fit" will "attempt to force the total visualization size to fit within the given width, height and padding values"
    Because the height/widthare set based on the parent container inupdateVegaSize(), this is there the problem arises if autosizeisn't set correctly. We need it to be set tofit` in order to prevent overflow/scrolling, and to fit everything within the container.

The vega (not vega lite) documentation discusses more about how to update the autosize value:

  • see details on autosize field / types here
  • see details on autosize being a reserved signal here (see "Built-in signals").

I can't find good examples of autosizing multi-view vega lite specs. But, similar to here, setting the autosize on the compiled vega spec seems to work. That example is listed in the above comment by calling autosize() signal on the vega view and updating to our desired autosize value.

@ohltyler
Copy link
Member

ohltyler commented Apr 13, 2023

To summarize, given the autosize documentation for vega (not vega-lite), there is no mention of limitations on multi-view/faceted displays not working, etc. That is because there is no notion of vconcat/hconcat/ etc. at the vega level.

So, if we set this setting after compilation in the vega spec via view.autosize(), it allows autosize to work correctly for our multi-view vconcat vega-lite spec.

@ohltyler
Copy link
Member

This will be resolved as part of #3415

@ohltyler
Copy link
Member

Closed as part of #3415

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working feature-anywhere unified visualization UX visualizations Issues and PRs related to visualizations
Projects
None yet
Development

No branches or pull requests

2 participants