Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Table] Fix Table sizing when shown in flex parent #1479

Merged
merged 6 commits into from
Aug 22, 2017
Merged

[Table] Fix Table sizing when shown in flex parent #1479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f260119
Style fixes when table is within a display: flex container
cmslewis Aug 21, 2017
ef46d67
Remove unnecessary rules
cmslewis Aug 21, 2017
c1cef0e
Clearer comment
cmslewis Aug 21, 2017
b2be1fe
Try to fix lint
cmslewis Aug 22, 2017
f1d50c7
Fix tests
cmslewis Aug 22, 2017
77c3408
Little CSS polishes
cmslewis Aug 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/table/src/common/classes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export const TABLE_QUADRANT_BODY_CONTAINER = "bp-table-quadrant-body-container";
export const TABLE_QUADRANT_LEFT = "bp-table-quadrant-left";
export const TABLE_QUADRANT_MAIN = "bp-table-quadrant-main";
export const TABLE_QUADRANT_SCROLL_CONTAINER = "bp-table-quadrant-scroll-container";
export const TABLE_QUADRANT_STACK = "bp-table-quadrant-stack";
export const TABLE_QUADRANT_TOP = "bp-table-quadrant-top";
export const TABLE_QUADRANT_TOP_LEFT = "bp-table-quadrant-top-left";
export const TABLE_REGION = "bp-table-region";
Expand Down
28 changes: 25 additions & 3 deletions packages/table/src/quadrants/_quadrants.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ $table-quadrant-z-index-top-left: $table-quadrant-z-index-left + 1;
// overflow within their parents; it should be large enough to hide a scrollbar of typical width.
$table-quadrant-scroll-container-overflow: 20px;

.bp-table-quadrant-stack {
display: flex;
height: 100%;
}

.bp-table-quadrant {
position: absolute;
top: 0;
Expand All @@ -26,7 +31,7 @@ $table-quadrant-scroll-container-overflow: 20px;

.bp-table-quadrant-scroll-container {
@include force-hardware-acceleration();
position: absolute;
position: relative;
top: 0;
right: 0;
bottom: 0;
Expand All @@ -51,9 +56,22 @@ $table-quadrant-scroll-container-overflow: 20px;
}

.bp-table-quadrant-main {
right: 0;
bottom: 0;
// use `relative` to ensure the table can size itself according to the MAIN quadrant's contents alone.
// other quadrants should remain `absolute` to ensure they're taken out of the document flow.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 this is usually my go-to solution for cutting my components out of the layout flow

position: relative;
top: auto;
left: auto;
z-index: $table-quadrant-z-index-main;
// the MAIN quadrant should fill the whole table so that ghost cells will be visible if enabled
width: 100%;
height: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

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

height: 100% can be tricky sometimes. just want to make sure this works correctly.

<div>
    <div style="height: 100%">everything is cool</div>
    <div>what now?</div>
</div>

this should be okay since we control the siblings of this div, but just something to keep an eye on.

Copy link
Contributor Author

@cmslewis cmslewis Aug 22, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Yep, I was cognizant of this. We're good as far as I can tell. Every other sibling is position: absolute.


.bp-table-quadrant-scroll-container {
// the MAIN quadrant's scroll bars should be visible and should be flush with the right and
// bottom edges of the table.
width: 100%;
height: 100%;
}

.bp-table-cell-client {
background: $white;
Expand All @@ -75,7 +93,11 @@ $table-quadrant-scroll-container-overflow: 20px;
z-index: $table-quadrant-z-index-left;

.bp-table-quadrant-scroll-container {
position: absolute;
top: 0;
right: -$table-quadrant-scroll-container-overflow;
bottom: 0;
height: auto;
overflow-x: hidden;
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/table/src/quadrants/tableQuadrantStack.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ export class TableQuadrantStack extends AbstractComponent<ITableQuadrantStackPro
const { grid, isRowHeaderShown, renderBody } = this.props;

return (
<div>
<div className={Classes.TABLE_QUADRANT_STACK}>
<TableQuadrant
bodyRef={this.props.bodyRef}
grid={grid}
Expand Down
20 changes: 13 additions & 7 deletions packages/table/test/tableTests.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ describe("<Table>", () => {
});

describe("Quadrants", () => {
// constrain the container to a smaller size to force scrolling
const CONTAINER_HEIGHT = 500;
const CONTAINER_WIDTH = 500;

const NUM_ROWS = 5;
const NUM_COLUMNS = 5;
const NUM_FROZEN_ROWS = 1;
Expand Down Expand Up @@ -562,13 +566,15 @@ describe("<Table>", () => {
document.body.appendChild(containerElement);

const tableComponent = ReactDOM.render(
<Table
numRows={LARGE_NUM_ROWS}
numFrozenColumns={NUM_FROZEN_COLUMNS}
numFrozenRows={NUM_FROZEN_ROWS}
>
{renderColumns({}, LARGE_NUM_COLUMNS)}
</Table>,
<div style={{ height: CONTAINER_HEIGHT, width: CONTAINER_WIDTH }}>
Copy link
Contributor

Choose a reason for hiding this comment

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

so, what happens if users DON'T put the table in an explicitly sized div? can we never use normal flow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the point of this PR actually: to fix that.

BEFORE (buggy):
image

AFTER (fluid height):
image

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, excellent.

<Table
numRows={LARGE_NUM_ROWS}
numFrozenColumns={NUM_FROZEN_COLUMNS}
numFrozenRows={NUM_FROZEN_ROWS}
>
{renderColumns({}, LARGE_NUM_COLUMNS)}
</Table>
</div>,
containerElement,
) as Table;

Expand Down