New Streams Table Column Sizing Fix #20141
Conversation
octavia-squidington-iv
added
area/platform
airbyte-webapp/src/components/SimpleTableComponents/SimpleTableComponents.tsx
Outdated
Show resolved
Hide resolved
| word-break: break-word; | ||
| color: ${({ theme, light, lighter }) => (light ? theme.greyColor40 : lighter ? theme.greyColor60 : "inherit")}; | ||
| font-weight: ${({ light, lighter }) => (light || lighter ? "normal" : "inherit")}; | ||
| color: ${({ theme, light }) => (light ? theme.greyColor60 : "inherit")}; |
There was a problem hiding this comment.
light was unused so I renamed lighter to light and removed the light functionality.
|
Did a timebox on repeating this on the old table. It's a touch more complex than the new table use-case and I don't feel it's worth more time. |
| @@ -9,7 +9,7 @@ import { useConnectionFormService } from "hooks/services/ConnectionForm/Connecti | |||
|
|
|||
| import styles from "./CatalogTreeSubheader.module.scss"; | |||
|
|
|||
| const SubtitleCell = styled(Cell).attrs(() => ({ lighter: true }))` | |||
| const SubtitleCell = styled(Cell).attrs(() => ({ light: true }))` | |||
There was a problem hiding this comment.
This was not caught by the rename! Can't wait to get rid of styled components.
…-table-column-sizing
…-table-column-sizing
| children, | ||
| }) => { | ||
| return <div className={classNames(styles.tableCell, sizeMap[size])}>{children}</div>; | ||
| }; |
There was a problem hiding this comment.
New cell component for these tables
| @@ -1,9 +1,10 @@ | |||
| .container { | |||
| margin-bottom: 29px; | |||
| max-height: 600px; | |||
| overflow-y: auto; | |||
| min-width: fit-content; | |||
There was a problem hiding this comment.
Absolutely core to this working.
|
|
||
| .tableCell { | ||
| flex: 0 0 $medium; | ||
| min-width: $medium; |
There was a problem hiding this comment.
Need to pair this with the flex-basis for it to work.
Looks like setting the CatalogTree to |
|
Tested it and found an issue with the old and new table: |
This issue exists in prod so I made a ticket. |
|
I tested again and it looks good despite the checkbox issue. One thing I noticed is that the table headers were always visible, but they're not anymore. Was this a conscious decision? |
This is likely due to needed to move the header into the same containing div as the body so the flex-width matched. So it was not explicitly done but is likely not an easy revert. |
…-table-column-sizing
airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss
Show resolved
Hide resolved
There was a problem hiding this comment.
Looks good. Tested both tables locally.
Discussed with @krishnaglick about not having sticky headers for now and making any other detail improvements separately.
…-table-column-sizing
What
Resolves #17937
How
Explicitly sets the widths for the columns, and prevents growing and shrinking.
Cleans up Cell component properties