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.

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

fix(component): prevent column resizing when editing text field #991

Merged
merged 1 commit into from
Sep 30, 2022

Conversation

jorgemoya
Copy link
Contributor

What?

Sets Worksheet to table-layout: fixed and add minWidth prop.

Fixes this issue introduced by #975:

Kapture 2022-09-28 at 12 01 48

Why?

The bug is happening because I set table-layout: auto, which means the table will try to automatically fit the content in the table (this is cool if we want all columns to remain visible even when shrinking the window), and overflow what doesn't fit (so we get a scroll bar).

Problem is that the input element is set to width: 100% and unless the td has a fixed width (set with the new width prop in the column), it won't calculate the 100% as I expected.

What I'm doing is reverting the table to table-layout: fixed. The problem with fixed and having static width columns it that when the table shrinks in size it will shrink all auto columns to 0 to accommodate the fixed width columns.

Like so:

Kapture 2022-09-28 at 10 46 43

To solve this I expose a minWidth prop for the Worksheet, so that we can set a minimum width. However this needs to be controlled by the developer, otherwise the issue above will happen.

Kapture 2022-09-28 at 10 49 35

Table will still have a static width if all columns have static widths.

Fixed inputs:

Kapture 2022-09-28 at 12 08 21

Overall this feels like a good compromise. We added the ability to set fixed widths to columns but by doing so developer needs to be mindful of setting a min-width if viewport gets too shrunk for the table. I believe minWidth was long overdue because the Worksheet has been breaking this way since it was introduced.

Testing/Proof

Locally.

@jorgemoya jorgemoya requested review from a team as code owners September 28, 2022 17:23
@jorgemoya jorgemoya requested review from bc-traciporter and bc-apostoliuk and removed request for bc-traciporter September 28, 2022 17:23
@@ -726,11 +727,11 @@ describe('SelectEditor', () => {

const cells = await findAllByDisplayValue('Value 1');

fireEvent.click(cells[0]);
await userEvent.click(cells[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

🙇🏼‍♀️

Copy link
Contributor

@bc-athorne bc-athorne left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@jorgemoya jorgemoya merged commit 66c0b68 into bigcommerce:master Sep 30, 2022
@jorgemoya jorgemoya deleted the fix-width-worksheet branch September 30, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants