Skip to content

Commit

Permalink
Issue 649 - Fix mismatched row height for fixed columns (#722)
Browse files Browse the repository at this point in the history
  • Loading branch information
Marc-Andre-Rivet committed Apr 14, 2020
1 parent 965f7ed commit aceddf3
Show file tree
Hide file tree
Showing 36 changed files with 1,212 additions and 788 deletions.
2 changes: 1 addition & 1 deletion packages/dash-table/.circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ version: 2
jobs:
"server-test":
docker:
- image: circleci/python:3.7.5-node-browsers
- image: circleci/python:3.7-node-browsers
- image: cypress/base:10

steps:
Expand Down
4 changes: 4 additions & 0 deletions packages/dash-table/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
All notable changes to this project will be documented in this file.
This project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]
### Fixed
- [#722](https://github.com/plotly/dash-table/pull/722) Fix a bug where row height is misaligned when using fixed_columns and/or fixed_rows

## [4.6.2] - 2020-04-01
### Changed
- [#713](https://github.com/plotly/dash-table/pull/713) Update from React 16.8.6 to 16.13.0
Expand Down
10 changes: 8 additions & 2 deletions packages/dash-table/demo/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,15 @@ export const generateMixedMarkdownMockData = (rows: number) => unpackIntoColumns
name: ['Markdown'],
type: ColumnType.Text,
presentation: 'markdown',
data: gendata(_ => [
data: gendata(i => [
'```javascript',
'console.warn("this is a markdown cell")',
...(i % 2 === 0 ?
['console.warn("this is a markdown cell")'] :
[
'console.log("logging things")',
'console.warn("this is a markdown cell")'
]
),
'```'].join('\n'), rows)
},
{
Expand Down
4 changes: 2 additions & 2 deletions packages/dash-table/src/dash-table/components/Cell/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export default class Cell extends Component<ICellProps> {
render() {
const {
attributes,
classes,
className,
onClick,
onDoubleClick,
onMouseEnter,
Expand All @@ -35,7 +35,7 @@ export default class Cell extends Component<ICellProps> {
ref='td'
children={(this as any).props.children}
tabIndex={-1}
className={classes}
className={className}
onClick={onClick}
onDoubleClick={onDoubleClick}
onMouseEnter={onMouseEnter}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ interface IAttributes {
export interface ICellProps {
active: boolean;
attributes: IAttributes;
classes: string;
className: string;
onClick: (e: MouseEvent) => void;
onDoubleClick: (e: MouseEvent) => void;
onMouseEnter: (e: MouseEvent) => void;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export default class CellDropdown extends PureComponent<IProps> {
if (applyFocus && dropdown && document.activeElement !== dropdown) {
// Limitation. If React >= 16 --> Use React.createRef instead to pass parent ref to child
const tdParent = DOM.getFirstParentOfType(dropdown.wrapper, 'td');
if (tdParent) {
if (tdParent && tdParent.className.indexOf('phantom-cell') === -1) {
tdParent.focus();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export default class CellMarkdown extends PureComponent<IProps, {}> {
if (applyFocus && el && document.activeElement !== el) {
// Limitation. If React >= 16 --> Use React.createRef instead to pass parent ref to child
const tdParent = DOM.getFirstParentOfType(el, 'td');
if (tdParent) {
if (tdParent && tdParent.className.indexOf('phantom-cell') !== -1) {
tdParent.focus();
}
}
Expand Down
192 changes: 102 additions & 90 deletions packages/dash-table/src/dash-table/components/ControlledTable/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,9 @@ const INNER_STYLE = {
minWidth: '100%'
};

const WIDTH_EPSILON = 0.5;
const MAX_WIDTH_ITERATIONS = 30;

export default class ControlledTable extends PureComponent<ControlledTableProps> {
private readonly menuRef = React.createRef<HTMLDivElement>();
private readonly stylesheet: Stylesheet = new Stylesheet(`#${this.props.id}`);
Expand Down Expand Up @@ -134,7 +137,7 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
setProps({ active_cell: selected_cells[0] });
}

this.applyStyle();
this.updateUiViewport();
this.handleResize();
}

Expand All @@ -146,7 +149,7 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>

componentDidUpdate() {
this.updateStylesheet();
this.applyStyle();
this.updateUiViewport();
this.handleResize();
this.handleDropdown();
this.adjustTooltipPosition();
Expand Down Expand Up @@ -226,6 +229,8 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>

handleResize = (force: boolean = false) => {
const {
fixed_columns,
fixed_rows,
forcedResizeOnly,
setState
} = this.props;
Expand All @@ -244,6 +249,7 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>

const { r0c0, r0c1, r1c0, r1c1 } = this.refs as { [key: string]: HTMLElement };


// Adjust [fixed columns/fixed rows combo] to fixed rows height
let trs = r0c1.querySelectorAll('tr');
Array.from(r0c0.querySelectorAll('tr')).forEach((tr, index) => {
Expand All @@ -261,12 +267,77 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
tr.style.height = getComputedStyle(tr2).height;
});

// Adjust fixed columns data to data height
const contentTd = r1c1.querySelector('tr > td:first-of-type');
if (contentTd) {
const contentTr = contentTd.parentElement as HTMLElement;
if (fixed_columns) {
const r1c0Table = r1c0.querySelector('table') as HTMLElement;
const r1c1Table = r1c0.querySelector('table') as HTMLElement;

r1c0Table.style.width = getComputedStyle(r1c1Table).width;

const lastVisibleTd = r1c0.querySelector(`tr:first-of-type > *:nth-of-type(${fixed_columns})`);

let it = 0;
let currentWidth = r1c0.getBoundingClientRect().width;
let lastWidth = currentWidth;

do {
lastWidth = currentWidth

// Force first column containers width to match visible portion of table
if (lastVisibleTd) {
const r1c0FragmentBounds = r1c0.getBoundingClientRect();
const lastTdBounds = lastVisibleTd.getBoundingClientRect();
currentWidth = lastTdBounds.right - r1c0FragmentBounds.left;

const width = `${currentWidth}px`;

r0c0.style.width = width;
r1c0.style.width = width;
}

// Force second column containers width to match visible portion of table
const firstVisibleTd = r1c1.querySelector(`tr:first-of-type > *:nth-of-type(${fixed_columns + 1})`);
if (firstVisibleTd) {
const r1c1FragmentBounds = r1c1.getBoundingClientRect();
const firstTdBounds = firstVisibleTd.getBoundingClientRect();

const width = firstTdBounds.left - r1c1FragmentBounds.left;
r0c1.style.marginLeft = `-${width}px`;
r0c1.style.marginRight = `${width}px`;
r1c1.style.marginLeft = `-${width}px`;
r1c1.style.marginRight = `${width}px`;
}

this.stylesheet.setRule('.dash-fixed-column tr', `height: ${getComputedStyle(contentTr).height};`);
it++;
} while (
Math.abs(currentWidth - lastWidth) > WIDTH_EPSILON ||
it < MAX_WIDTH_ITERATIONS
)
}

if (fixed_columns || fixed_rows) {
const r1c0CellWidths = Array.from(
r1c0.querySelectorAll('table.cell-table > tbody > tr:first-of-type > *')
).map(c => c.getBoundingClientRect().width);

const r1c1CellWidths = Array.from(
r1c1.querySelectorAll('table.cell-table > tbody > tr:first-of-type > *')
).map(c => c.getBoundingClientRect().width);

Array.from<HTMLElement>(
r0c0.querySelectorAll('table.cell-table > tbody > tr:first-of-type > *')
).forEach((c, i) => this.setCellWidth(c, r1c0CellWidths[i]));

Array.from<HTMLElement>(
r0c0.querySelectorAll('table.cell-table > tbody > tr:last-of-type > *')
).forEach((c, i) => this.setCellWidth(c, r1c0CellWidths[i]));

Array.from<HTMLElement>(
r0c1.querySelectorAll('table.cell-table > tbody > tr:first-of-type > *')
).forEach((c, i) => this.setCellWidth(c, r1c1CellWidths[i]));

Array.from<HTMLElement>(
r0c1.querySelectorAll('table.cell-table > tbody > tr:last-of-type > *')
).forEach((c, i) => this.setCellWidth(c, r1c1CellWidths[i]));
}
}

Expand Down Expand Up @@ -627,98 +698,20 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
) || page_action === TableAction.Custom;
}

applyStyle = () => {
const {
fixed_columns,
fixed_rows,
row_deletable,
row_selectable
} = this.props;

const { r1c0, r1c1 } = this.refs as { [key: string]: HTMLElement };

this.updateUiViewport();

if (row_deletable) {
this.stylesheet.setRule(
`.dash-spreadsheet-inner td.dash-delete-cell`,
`width: 30px; max-width: 30px; min-width: 30px;`
);
}

if (row_selectable) {
this.stylesheet.setRule(
`.dash-spreadsheet-inner td.dash-select-cell`,
`width: 30px; max-width: 30px; min-width: 30px;`
);
}

// Adjust the width of the fixed row header
if (fixed_rows) {
Array.from(r1c1.querySelectorAll('tr:first-of-type td.dash-cell, tr:first-of-type th.dash-header')).forEach(td => {
const classname = td.className.split(' ')[1];
const style = getComputedStyle(td);
const width = style.width;

this.stylesheet.setRule(
`.dash-fixed-row:not(.dash-fixed-column) th.${classname}`,
`width: ${width} !important; min-width: ${width} !important; max-width: ${width} !important;`
);
});
}

// Adjust the width of the fixed row / fixed columns header
if (fixed_columns && fixed_rows) {
Array.from(r1c0.querySelectorAll('tr:first-of-type td.dash-cell, tr:first-of-type th.dash-header')).forEach(td => {
const classname = td.className.split(' ')[1];
const style = getComputedStyle(td);
const width = style.width;

this.stylesheet.setRule(
`.dash-fixed-column.dash-fixed-row th.${classname}`,
`width: ${width} !important; min-width: ${width} !important; max-width: ${width} !important;`
);
});
}

// Adjust widths of row deletable/row selectable headers
const subTable = fixed_rows && !fixed_columns ? r1c1 : r1c0;

if (row_deletable) {
Array.from(subTable.querySelectorAll('tr:first-of-type td.dash-delete-cell')).forEach(td => {
const style = getComputedStyle(td);
const width = style.width;

this.stylesheet.setRule(
'th.dash-delete-header',
`width: ${width} !important; min-width: ${width} !important; max-width: ${width} !important;`
);
});
}
if (row_selectable) {
Array.from(subTable.querySelectorAll('tr:first-of-type td.dash-select-cell')).forEach(td => {
const style = getComputedStyle(td);
const width = style.width;

this.stylesheet.setRule(
'th.dash-select-header',
`width: ${width} !important; min-width: ${width} !important; max-width: ${width} !important;`
);
});
}
}

handleDropdown = () => {
const { r1c1 } = this.refs as { [key: string]: HTMLElement };

dropdownHelper(r1c1.querySelector('.Select-menu-outer'));
}

onScroll = (ev: any) => {
const { r0c1 } = this.refs as { [key: string]: HTMLElement };
const { r0c0, r0c1 } = this.refs as { [key: string]: HTMLElement };

Logger.trace(`ControlledTable fragment scrolled to (left,top)=(${ev.target.scrollLeft},${ev.target.scrollTop})`);
r0c1.style.marginLeft = `${-ev.target.scrollLeft}px`;

const margin = parseFloat(ev.target.scrollLeft) + parseFloat(r0c0.style.width);

r0c1.style.marginLeft = `${-margin}px`;

this.updateUiViewport();
this.handleDropdown();
Expand Down Expand Up @@ -952,6 +945,25 @@ export default class ControlledTable extends PureComponent<ControlledTableProps>
}
}

private setCellWidth(cell: HTMLElement, width: number) {
cell.style.width = `${width}px`;
cell.style.minWidth = `${width}px`;
cell.style.maxWidth = `${width}px`;
cell.style.boxSizing = 'border-box';

/**
* Some browsers handle `th` and `td` size inconsistently.
* Checking the size delta and adjusting for it (different handling of padding and borders)
* allows the table to make sure all sections are correctly aligned.
*/
const delta = cell.getBoundingClientRect().width - width;
if (delta) {
cell.style.width = `${width - delta}px`;
cell.style.minWidth = `${width - delta}px`;
cell.style.maxWidth = `${width - delta}px`;
}
}

private get showToggleColumns(): boolean {
const {
columns,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import TableClipboardHelper from 'dash-table/utils/TableClipboardHelper';
type SetFilter = (ev: any) => void;

interface IColumnFilterProps {
classes: string;
className: string;
columnId: ColumnId;
isValid: boolean;
setFilter: SetFilter;
Expand Down Expand Up @@ -39,15 +39,15 @@ export default class ColumnFilter extends PureComponent<IColumnFilterProps, ISta

render() {
const {
classes,
className,
columnId,
isValid,
style,
value
} = this.props;

return (<th
className={classes + (isValid ? '' : ' invalid')}
className={className + (isValid ? '' : ' invalid')}
data-dash-column={columnId}
style={style}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export default class FilterFactory {

return (<ColumnFilter
key={`column-${index}`}
classes={`dash-filter column-${index}`}
className={`dash-filter column-${index}`}
columnId={column.id}
isValid={!ast || ast.isValid}
setFilter={this.onChange.bind(this, column, map, setFilter)}
Expand Down
Loading

0 comments on commit aceddf3

Please sign in to comment.