search

finos / perspective

A data visualization and analytics component, especially well-suited for large and/or streaming datasets.
https://perspective.finos.org/
Apache License 2.0
7.72k stars 1.04k forks source link

reenable minimum height for voila #2588

Closed timkpaine closed 1 month ago

timkpaine commented 2 months ago

Tweaks and partially reverts https://github.com/finos/perspective/pull/2352

Currently, Perspective works great in voila except that it does not have any height::

Screenshot 2024-04-05 at 17 57 46

This PR gives it a minimum height:

Screenshot 2024-04-05 at 17 57 26

The sentiments in the original PR still hold, I'm looking for an alternative way to tell if specifically in voilà. but with https://github.com/voila-dashboards/voila/pull/1457 we can now disambiguate and apply only in voilà

timkpaine commented 2 months ago
timkpaine commented 2 months ago

Not done in this PR, but we could use the same idea for notebook, which might help if/when we work in e.g. vs code: https://github.com/jupyter/notebook/blob/3b8e6736075279f80cd1e590c71a872dc5ad2c11/notebook/templates/notebooks.html#L15

timkpaine commented 2 months ago

needs https://github.com/finos/perspective/pull/2596

timkpaine commented 1 month ago

In voila (and vs code and jupyterlab and Jupyter notebook for that matter) the output container expands to the size of its contents, so if you just set height: 100% you'll end up with a height 0 div since perspective tries to expand to fill its container and does not in-and-of-itself have a minimum height. Thus we need to give the container div some minimum height so that it displays, and 520px seemed to be a good heuristic from a UX standpoint (e.g. its pretty close to the default pandas max height when it collapses intermediate rows).

You can run a voila app with voila <notebook.ipynb> and it will run locally, I don't think it really requires tests since for all these various platforms we just need to find the container element and set a height next to the other container blocks.

Your change is fine but I think there is a reason for how we do it (though I am not aware of the reason 😄). I know we set height 98% in the viewer so we have room for that resize part (in case someone wants to make the perspective widget larger than 520px). Im ok with setting an absolute div.PSPContainer { height: 520px } but im unsure of the implications across outputs we don't know about (e.g. other than Jupyter Notebook, JupyterLab, Voilà, and VS Code, which are the 4 we've been tracking).

texodus commented 1 month ago

I've cherry-picked this into #2623, closing in favor of this PR.