Skip to content

Refactor Table to use flex layouts #2622

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 3 commits into from
Dec 11, 2024

Conversation

viktorrenkema
Copy link
Contributor

@viktorrenkema viktorrenkema commented Dec 11, 2024

This PR changes the way we render Tables. It now implements a strategy closer to the GitBook in-app editor, where we use flexbox for layout instead of semantic <table> <tr> etc, and use role on divs for these instead.

The change was motivated by limitations of layout with html table elements. Our columns can be sized in 3 different ways:
1. Set to auto-size by default, these columns share the available width
2. Explicitly set by the user by dragging column separator (we then turn off auto-size)
3. Auto-size is turned off without setting a width, we then default to a fixed width of 100px

This lead to a bunch of layout headaches, like:

  • if you have the <table> take up width: 100% of the available space, columns would always increase in size when possible, even though if we/users explicitly set a custom width on a column through dragging the column separator
  • we can use width: fit-content, which also allows for tables in smaller width than the page (altho this is currently broken, but then any columns with no set width should enforce auto-sizing. but this is not possible without measuring the DOM (inefficient), as the parent is set to fit-content, not 100%

Ultimately using flexbox gives us a lot more flexibility and potential for adding more features, and we're closer to the editor.

This PR also fixes a number of bugs:

Copy link

linear bot commented Dec 11, 2024

Copy link

changeset-bot bot commented Dec 11, 2024

⚠️ No Changeset found

Latest commit: c001411

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

github-actions bot commented Dec 11, 2024

GitBook Preview
Latest commit: https://50d845b2.gitbook-open.pages.dev
PR: https://pr2622.gitbook-open.pages.dev

Copy link

argos-ci bot commented Dec 11, 2024

The latest updates on your projects. Learn more about Argos notifications ↗︎

Build Status Details Updated (UTC)
default (Inspect) 👍 Changes approved 21 changed Dec 31, 2024, 11:12 PM

Copy link
Member

@SamyPesse SamyPesse left a comment

Choose a reason for hiding this comment

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

Looks good, but I'm not a CSS expert

@viktorrenkema viktorrenkema force-pushed the viktor/rnd-3264/table-widths-to-flex branch from b8fa4d4 to 2ff1075 Compare December 11, 2024 16:21
@viktorrenkema viktorrenkema merged commit 60e3500 into main Dec 11, 2024
9 checks passed
@viktorrenkema viktorrenkema deleted the viktor/rnd-3264/table-widths-to-flex branch December 11, 2024 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants