Skip to content

Properly update TableView row width when it is the last column remaining #2028

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.

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

Merged
merged 1 commit into from
Jun 16, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
51 changes: 51 additions & 0 deletions packages/@react-spectrum/table/test/Table.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3809,6 +3809,57 @@ describe('TableView', function () {
expect(within(row).getAllByRole('gridcell')).toHaveLength(5);
}
});

it('should update the row widths when removing and adding columns', function () {
function compareWidths(row, b) {
let newWidth = row.childNodes[1].style.width;
expect(parseInt(newWidth, 10)).toBeGreaterThan(parseInt(b, 10));
return newWidth;
}

let tree = render(<HidingColumns />);
let table = tree.getByRole('grid');
let columns = within(table).getAllByRole('columnheader');
expect(columns).toHaveLength(6);

let rows = tree.getAllByRole('row');
let oldWidth = rows[1].childNodes[1].style.width;

let audienceCheckbox = tree.getByLabelText('Audience Type');
let budgetCheckbox = tree.getByLabelText('Net Budget');
let targetCheckbox = tree.getByLabelText('Target OTP');
let reachCheckbox = tree.getByLabelText('Reach');

userEvent.click(audienceCheckbox);
expect(audienceCheckbox.checked).toBe(false);
act(() => jest.runAllTimers());
oldWidth = compareWidths(rows[1], oldWidth);

userEvent.click(budgetCheckbox);
expect(budgetCheckbox.checked).toBe(false);
act(() => jest.runAllTimers());
oldWidth = compareWidths(rows[1], oldWidth);

userEvent.click(targetCheckbox);
expect(targetCheckbox.checked).toBe(false);
act(() => jest.runAllTimers());
oldWidth = compareWidths(rows[1], oldWidth);

// This previously failed, the first column wouldn't update its width
// when the 2nd to last column was removed
userEvent.click(reachCheckbox);
expect(reachCheckbox.checked).toBe(false);
act(() => jest.runAllTimers());
oldWidth = compareWidths(rows[1], oldWidth);
columns = within(table).getAllByRole('columnheader');
expect(columns).toHaveLength(2);

// Readd the column and check that the width decreases
userEvent.click(audienceCheckbox);
expect(audienceCheckbox.checked).toBe(true);
act(() => jest.runAllTimers());
expect(parseInt(rows[1].childNodes[1].style.width, 10)).toBeLessThan(parseInt(oldWidth, 10));
});
});

describe('headerless columns', function () {
Expand Down
3 changes: 2 additions & 1 deletion packages/@react-stately/virtualizer/src/Virtualizer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -867,7 +867,8 @@ export class Virtualizer<T extends object, V, W> {

let layoutInfo = this.layout.getLayoutInfo(cur.key);
if (
!cur.rect.pointEquals(layoutInfo.rect) ||
// Uses equals rather than pointEquals so that width/height changes are taken into account
!cur.rect.equals(layoutInfo.rect) ||
Comment on lines +870 to +871
Copy link
Member Author

Choose a reason for hiding this comment

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

If all columns were removed/hidden except the left most column, the x, y position of the rect doesn't shift but the width changes. Changed to rect.equals instead of rect.pointEquals so that this case is properly captured

cur.opacity !== layoutInfo.opacity ||
cur.transform !== layoutInfo.transform
) {
Expand Down