datatree icon indicating copy to clipboard operation
datatree copied to clipboard

Rich display width is broken

Open eschalkargans opened this issue 1 year ago • 4 comments

Bug description

Screenshot from 2024-05-16 17-58-24

The group width is shrinked, not using the full available width. The width should only be restrained as we progress in the hierarchy, making high level groups almost the same size as classical datasets

We can see more clearly the width issue in this high contrast screenshot:

Screenshot from 2024-05-16 18-03-44

Other bugs

Note: the alternating colors for the data variables rows seem not to take into account the theme. They should be dependent on the them, like the rest of the rich display

eschalkargans avatar May 16 '24 16:05 eschalkargans

Technical analysis

Quick fix

The quick and dirty fix can be the following

-  "<div class='xr-wrap' style='display:none'>"
+ "<div class='xr-wrap' style='display:none; min-width: 700px;'>"

It forces override the min-width: 300px coming from the xr-wrap CSS class, and replace it with the same value as its max-width of 700px.

See the method _obj_repr https://github.com/pydata/xarray/blob/12123be8608f3a90c6a6d8a1cdc4845126492364/xarray/core/formatting_html.py#L297C22-L297C24

See the xr-wrap class https://github.com/pydata/xarray/blob/12123be8608f3a90c6a6d8a1cdc4845126492364/xarray/static/css/style.css#L29

Better fix

The good fix would be to update the class xr-wrap ; however this class is from the xarray's core, so changing it would not only change datatree but xarray too. So, an alternative could be for instance, adding a new class, xr-datatree-wrap, with more-suited width parameters, copied from xr-wrap with a minor change:

.xr-wrap {
  display: block !important;
  min-width: 300px;
  max-width: 700px;
}

.xr-dataset-wrap {
  display: block !important;
  min-width: 700px;
  max-width: 700px;
}

Improvement

The following applies to both xarray and datatree.

The current collapsible section is based on using hidden checkboxes with the CSS :checked selector to reveal or hide the elements. This code is from 5 years ago. This StackOverflow thread shows the current solution being using <details>, and the old solution of using hidden checkboxes and CSS :checked selection as an older solution: https://stackoverflow.com/questions/41220717/collapse-without-javascript

Recently, a new couple of HTML elements have been added, <details> and <summary>, that are the official HTML tags to implemented collapsible sections with little effort. See MDN docs: <details>: The Details disclosure element (widely available on browsers according to this link)

Example (it works with GitHub's markdown)

  • Giant planets
    • Gas giants
      • Jupiter
      • Saturn
    • Ice giants
      • Uranus
      • Neptune

Example taken from Tree views in CSS

The idea would be to just create a tree structure of nested <section> and <summary>, and delegating the final HTML rendering to the current Dataset code. This can be done for each node, by using to_dataset(), whether the group is a leaf or not. Each node would be: its Dataset (the leaf variables it contains) + its children.

An other alternative, at the cost of a dependency, would be to use ipytree, a Jupyter widget representing a tree. This is a Jupyter widget ; can it be rendered as HTML for export?

etienneschalk avatar May 20 '24 14:05 etienneschalk

This should be a duplicate of #91 (unless I'm misunderstanding something?). Can you try to reproduce with xarray.core.datatree.DataTree? I believe this should have been fixed in pydata/xarray#8930.

keewis avatar May 20 '24 14:05 keewis

Hello, indeed it is a duplicate of #91 and is fixed when using xarray.core.datatree.DataTree!

The point of using <details> and <summary> remains, but as long as the current solution works... I am fine with it!

Is it planned to backport the bugfixes of xarray to this datatree repo, or not (so users are encouraged to use the latest xarray versions)?

etienneschalk avatar May 20 '24 16:05 etienneschalk

Is it planned to backport the bugfixes of xarray to this datatree repo, or not (so users are encouraged to use the latest xarray versions)?

No - this repo will be archived in favour of using xarray itself.

TomNicholas avatar May 20 '24 18:05 TomNicholas