🪟 🎨 Streams table design pass [with fix] #19431
Conversation
octavia-squidington-iv
added
area/platform
teallarson
changed the title
|
Confirmed visually that this fixes the arrow bug |
teallarson
changed the title
There was a problem hiding this comment.
When testing, a couple of issues while testing the existing table. It looks like because of the flushing of the card, some of the padding was lost around the table. You can see in this screenshot that the + and - icons are too close to the edge and on the right side the row does not have the padding it had before:
This is master:
airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss
Outdated
Show resolved
Hide resolved
| @@ -5,16 +5,37 @@ $icon-size: 15px; | |||
| .container { | |||
| display: flex; | |||
| flex-direction: row; | |||
| justify-content: space-between; | |||
| padding: variables.$spacing-lg calc(variables.$spacing-lg * 2); | |||
| padding: variables.$spacing-xl calc(#{variables.$spacing-lg} * 2) 0; | |||
There was a problem hiding this comment.
Same suggestion as above. No need to use the calc since it will appear in css as calc(10px * 2) instead of 20px when adding sass vars.
There was a problem hiding this comment.
Thanks! For my own understanding.... the right time to use calc is when you're referring to something outside of the css vars as part of the expression (like 10%)... but if you're just evaluating something with the variables and a static number, simply put the variable?
|
I also noticed that the new table header is way off with the new table width. I am OK fixing this separately: |
…logTreeTableRow.module.scss Co-authored-by: Edmundo Ruiz Ghanem <[email protected]>
|
I left #17937 open with a note RE: the headers. |
|
Addressed offline feedback RE: alignment of the search bar and card heading for the sync catalog on the "classic view" + 1 more item (the left padding on icons for an enabled/disabled row) on the "new view". |
| @@ -583,6 +583,9 @@ exports[`CreateConnectionForm should render 1`] = ` | |||
| <div | |||
| class="<removed-for-snapshot-test>" | |||
| > | |||
| <div | |||
There was a problem hiding this comment.
extra div is because i fixed some bad alignment in the header
| @@ -530,6 +530,9 @@ exports[`ConnectionReplicationTab should render 1`] = ` | |||
| <div | |||
| class="<removed-for-snapshot-test>" | |||
| > | |||
| <div | |||
There was a problem hiding this comment.
same as other snapshot file... added this div to fix header alignment some
teallarson
deleted the
revert-19397-revert-18908-teal/streams-table-design-pass
branch
* Revert "Revert "🪟 🎨 Streams table design pass (#18908)" (#19397)" This reverts commit ebbe5a9. * remove min-width 0 that broke the table * Update airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.module.scss Co-authored-by: Edmundo Ruiz Ghanem <[email protected]> * fix styling for master table with (padding) * fix alignment somewhat on master view * fix padding differences between old/new tables * fix icon alignment in new table rows * update snapshots Co-authored-by: Edmundo Ruiz Ghanem <[email protected]>
Reverts #19397 and removes the broken CSS from the original PR.
When the stream tables design pass was originally merged, there was an unneeded
min-width: 0that caused the fields panel dropdown arrow to disappear on the master table (IE: not the one hidden behind the .env variable)This PR removes that
min-width: 0It does not cause and visual changes to the main table, but does re-introduce the styles for our new sync catalog table!