Skip to content
This repository was archived by the owner on Aug 29, 2025. It is now read-only.

Issue 118 width behavior#129

Merged
Marc-Andre-Rivet merged 4 commits into
masterfrom
issue-118-width-behavior
Oct 5, 2018
Merged

Issue 118 width behavior#129
Marc-Andre-Rivet merged 4 commits into
masterfrom
issue-118-width-behavior

Conversation

@Marc-Andre-Rivet

Copy link
Copy Markdown
Contributor
  • allow usage of width % for standard and fixed columns/rows contexts
  • make sure fixed rows and aligned with the rest of the table
  • visual tests for behavior above

Marc-André Rivet added 2 commits October 3, 2018 16:24
@chriddyp
chriddyp temporarily deployed to dash-table-review-pr-129 October 3, 2018 20:42 Inactive
Comment thread CHANGELOG.md

For the percentages to have meaning, the dash-table must be forced to have a width and the content of the dash-table must be forced to grow to fill the available space made available by the container (by default the table is only as big as it needs to be).

Added prop content_style that takes values 'fit' or 'grow' (Default='fit')

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Default table behavior is to take as little space as possible (fit) but for % based columns and it sometimes (always?) desirable to fill the dash-table container (grow)

`.cell-0-1 th:nth-of-type(${index + 1})`,
`width: ${width}; min-width: ${width}; max-width: ${width};`
);
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure the header rows match the size (pixel for pixel) of the content rows.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is necessary for fixed rows, as the rows are not in the same table and need to be synced

this.stylesheet.setRule(
`.dash-spreadsheet-inner th.column-${typeIndex}`,
`width: ${width}; max-width: ${maxWidth}; min-width: ${minWidth};`
);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These styles specific to headers are not necessary anymore as the more general styles above will have the same effect.

n_fixed_columns ? 'dash-fixed-column' : '',
'dash-fixed-content'
]
];

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply dash-* classes that are part of the table contract to the table fragments that are applicable

...(n_fixed_rows ? ['freeze-top'] : []),
...(n_fixed_columns ? ['freeze-left'] : [])
...(n_fixed_columns ? ['freeze-left'] : []),
[`dash-${content_style}`]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dash-grow or dash-fit, also contract material

const width = `calc(${spannedColumns.map(column => Stylesheet.unit(column.width || DEFAULT_CELL_WIDTH, 'px')).join(' + ')})`;
const maxWidth = `calc(${spannedColumns.map(column => Stylesheet.unit(column.maxWidth || column.width || DEFAULT_CELL_WIDTH, 'px')).join(' + ')})`;
const minWidth = `calc(${spannedColumns.map(column => Stylesheet.unit(column.minWidth || column.width || DEFAULT_CELL_WIDTH, 'px')).join(' + ')})`;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these are not applicable anymore and taken care of by the new sync style above

table {
width: 100%;
}
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When content_style='grow' we want the right side of the table (non-frozen columns) to fill the available space.
We also want all tables to grow as well

@chriddyp
chriddyp temporarily deployed to dash-table-review-pr-129 October 4, 2018 11:42 Inactive

@valentijnnieman valentijnnieman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@Marc-Andre-Rivet

Marc-Andre-Rivet commented Oct 5, 2018

Copy link
Copy Markdown
Contributor Author

@cldougl Code approval from @valentijnnieman. Will wait on your functional review before merging 😄

Preliminary testing in app repo seem to be working but I haven't been able to try different datasets

content_style='grow'
table_style=[{ selector: '.dash-spreadsheet', rule: 'width: '100%' }]
columns=[..., { ..., width: '40%', ... }, ...]

@cldougl cldougl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃 lgtm

@Marc-Andre-Rivet
Marc-Andre-Rivet merged commit 087e9ba into master Oct 5, 2018
@Marc-Andre-Rivet
Marc-Andre-Rivet deleted the issue-118-width-behavior branch October 12, 2018 13:19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants