search

jupyter / dashboards_server

[RETIRED] Server that runs and renders Jupyter notebooks as interactive dashboards
Other
181 stars 48 forks source link

Fix extraneous cell padding #246

Closed parente closed 8 years ago

parente commented 8 years ago

Dashboard preview in the 0.5.2 layout extension (which uses Gridstack) does not include any extra padding on cells:

screen shot 2016-05-24 at 9 21 48 am

Our read-only Gridstack-light clone here in dashboard server dynamically adds padding which causes full cells to overflow and misalign (see right edge of table):

screen shot 2016-05-24 at 9 21 06 am

This PR removes the extra padding. @jhpedemonte, @dalogsdon any reason why it should remain?

dalogsdon commented 8 years ago

The padding is based on the cell margin, so it could have something to do with the space between cells. However, in this case it looks wrong. Make sure removing it preserves the apparent space between cells in a grid.

parente commented 8 years ago

Make sure removing it preserves the apparent space between cells in a grid.

I'll dig a bit more into what is providing the space in the gridstack case today vs what dashboard server is doing.

parente commented 8 years ago

Added a test_padding notebook. Here it is in notebook server with jupyter_dashboards 0.5.2:

screen shot 2016-05-26 at 9 28 28 am

Here it is in dashboard server WITH the fix in this PR:

screen shot 2016-05-26 at 9 29 14 am

Here it is in dashboard server WITHOUT the fix in this PR:

screen shot 2016-05-26 at 9 36 35 am

Note the excessive left-right padding without the fix.

dalogsdon commented 8 years ago

I am not sure completely removing the padding is the correct solution. In the notebook, we add margin to the inner-cell container to visually add space between each cell (seen on hover when authoring dashboard). The padding in dashboard server was an attempt to mimic that so the spacing is identical, so perhaps it just needed to be modified rather than removed.

In the colors example above (with @parente's fix), I think it just happens to look the same between the notebook and the dashboard server because the phosphor widgets have their own padding that happens to match the cell margin we were adding in the notebook. This exposes another issue since the notebook is not yet using the new JS libs that use Phosphor widgets. Perhaps this was the original issue in the first place instead of our padding being wrong.

I made another example notebook using Markdown cells containing images and observed a spacing difference (with @parente's fix):

Notebook: image

Dashboard server: image

parente commented 8 years ago
  1. That's a scary set of images.
  2. Let's pair on seeing if modification rather than removal corrects all cases. There is #227 about the phosphor differences which I think are separate.