search

plotly / dash-table

OBSOLETE: now part of https://github.com/plotly/dash
https://dash.plotly.com
MIT License
420 stars 72 forks source link

Issue 777 - Align column headers when using fixed rows #793

Closed Marc-Andre-Rivet closed 4 years ago

Marc-Andre-Rivet commented 4 years ago

Fixes #777 Fixes #780

Under certain circumstances, the table fragments were misaligned for scenarios involving fixed_rows and fixed_columns. Also, some visual tests were already showing that the behavior was incorrect but it was hard to spot unless zooming in. For that reason I believe the tests already appropriately cover the cases for this PR and that no additional tests are required, only more caution.

A few problems were encountered:

  1. Certain browsers require both the tr and the th rows to have their size explictly set when forcing them through styles. This happens for fixed row/columns using the data nested field. For example, in Firefox, forcing a tr to have a width of 200px can be overriden by a previous th requiring more space. The th "wins". The fix for this is to both set the tr and th cell size for the last row of each group.

  2. The implementation was subtly wrong, using the size of the wrong fragment to coerce another. I think some of those is due to historical reasons, and were not reworked correctly when the table was reworked to allow variable row heights in https://github.com/plotly/dash-table/pull/722.

  3. Certain browsers, for certain tables will not be able to converge on a single table width, but will instead oscillate between two very close values. As far as I know always distant by 1px. The "iterations" code for the table now checks for cycles.

Also, a nice side-effect of this PR is that tables with fixed columns now behave more consistently vs. unfixed ones. Will need to check the documentation for any mention of incorrect width that will have to be removed. image now looks like this fixed and unfixed: image

Marc-Andre-Rivet commented 4 years ago

@alexcjohnson Addressed all previous comments. Also added https://github.com/plotly/dash-table/pull/793/commits/b02a0226a00d2083caf8ec6e1e5fd8bc698ff927 to fix #780 and lock behavior with new tests.