🪟 🎨 Streams table design pass #18908

teallarson · 2022-11-03T14:03:57Z

What

addresses most of #17937
Design updates for the Connection Streams Table for Auto Detect Schema Changes.

Figma prototypes used:

sync mode dropdown based on this prototype
most of the rest of it based on this prototype

Not addressed:

not all of the background color changes match the prototype 100% (tracked separately in Highlighting of rows in syncCatalog table #19092)
tooltip for the syncmode dropdown (tracked separately in Update new connection streams table dropdowns #18243)
table header alignment

How

A lot of sass updates. Not many changes beyond that aside from a wrapper component for TextCells.

Note:

To check this out locally, make sure you have REACT_APP_NEW_STREAMS_TABLE=true in your .env.development file.

teallarson · 2022-11-04T20:24:10Z

airbyte-webapp/src/components/connection/CatalogTree/CatalogTreeBody.module.scss

@@ -1,7 +1,7 @@
 .container {
  margin-bottom: 29px;
  max-height: 600px;
-  overflow-y: auto;
+  overflow-y: overlay;


note: this works well on chrome on a mac... but is not compatible with firefox. should find another solution :(

Let's follow up on this and figure out what we can do.

solved using scrollbar-gutter. Still wonky on Safari if a user has an external mouse, but that seemed a reasonable tradeoff to Tim and I.

teallarson · 2022-11-04T20:29:21Z

airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableHeader.tsx

-      </Cell>
-      <Cell>
+      </TextCell>
+      {/* <TextCell>
        <FormattedMessage id="form.fields" />


I saw the field count looks like it will be in the design in a future round. Leaving this here.

teallarson · 2022-11-04T20:30:06Z

airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx

        <Switch small checked={stream.config?.selected} onChange={onSelectStream} disabled={disabled} />
      </Cell>
-      <Cell>{fieldCount}</Cell>
-      <HeaderCell ellipsis title={stream.stream?.namespace || ""}>
+      {/* <Cell>{fieldCount}</Cell> */}


field counts look like they're coming back in a future iteration, leaving this here for now... there may need to be adjustments to the flex properties in the cells and header(s) as a result.

teallarson · 2022-11-04T20:32:45Z

airbyte-webapp/src/components/connection/CatalogTree/next/StreamConnectionHeader.module.scss

+}
+
+.source {
+  flex: 7.3 0 0;


The .3 is an unfortunate awkward number. This value is the sum of the flex values + that .3 "buffer" for the gaps.

teallarson · 2022-11-05T17:06:11Z

airbyte-webapp/src/components/connection/CatalogTree/next/StreamConnectionHeader.tsx

-      <FontAwesomeIcon icon={faArrowRight} />
-      <div className={styles.connector}>{renderIcon(destinationDefinition.icon)} Destination</div>
+      <div className={sourceStyles}>
+        {renderIcon(sourceDefinition.icon)}{" "}


The icon styles predate this PR and match the values in Figma. However, some logos appear very small (Salesforce for one). I'm in favor of adjusting it, but want to see if others agree before I go straying from the designs.

Looking at the preview, the logos look too small for me because of the irregular size. Either the width or width is consistently 15 px or the height but not both because it will make the icon shrink to fit both. Another difference with Figma is that the icons will not render as sharply as they do, so for example the Postgres icon doesn't look great there. We may need to revisit this with design to look acceptable in the implementation.

teallarson · 2022-11-05T17:20:37Z

airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx

@@ -63,14 +63,13 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({
    [styles.disabledChange]: changedSelected && !isStreamEnabled,
    [styles.selected]: isSelected,
    [styles.error]: hasError,
+    [styles.disabled]: !changedSelected && !isStreamEnabled,


Note that this makes the syncmode dropdown blend into the background. I do see this in the designs as well, and users can't save changes to a disabled stream anyways currently... so maybe that's ok? Since they can't change things, I wonder if we want to disable the dropdowns in these rows though.

I noticed that! I'm not a big fan, and maybe we can work around it by disabling it. Additionally, I feel the color contrast may be too low and we may need to revisit this with design.

edmundito

Really great to see the table coming together! Followed up on some of your questions and pointed some things I noticed during testing.

edmundito · 2022-11-09T14:28:51Z

airbyte-webapp/src/components/connection/CatalogTree/next/SyncModeSelect.module.scss

 .pillSelect {
-  width: 100%;
+  width: variables.$width-wide-menu;


100% did not work as expected?

It did not. Also, Nico pointed out that the SyncModeSelect should stay consistent width even when the other columns resize.

airbyte-webapp/src/views/Connection/ConnectionForm/components/SyncCatalogField.module.scss

edmundito · 2022-11-09T14:47:37Z

airbyte-webapp/src/components/connection/CatalogTree/next/StreamConnectionHeader.module.scss

  width: $icon-size;
+  height: $icon-size;
+  padding-right: variables.$spacing-sm;


Could this be set as a gap property in the source/destination style?

edmundito · 2022-11-09T14:51:37Z

airbyte-webapp/src/components/connection/CatalogTree/next/StreamConnectionHeader.tsx

-      <FontAwesomeIcon icon={faArrowRight} />
-      <div className={styles.connector}>{renderIcon(destinationDefinition.icon)} Destination</div>
+      <div className={sourceStyles}>
+        {renderIcon(sourceDefinition.icon)}{" "}


Looking at the preview, the logos look too small for me because of the irregular size. Either the width or width is consistently 15 px or the height but not both because it will make the icon shrink to fit both. Another difference with Figma is that the icons will not render as sharply as they do, so for example the Postgres icon doesn't look great there. We may need to revisit this with design to look acceptable in the implementation.

edmundito · 2022-11-09T14:53:56Z

airbyte-webapp/src/components/connection/CatalogTree/CatalogTreeBody.module.scss

@@ -1,7 +1,7 @@
 .container {
  margin-bottom: 29px;
  max-height: 600px;
-  overflow-y: auto;
+  overflow-y: overlay;


Let's follow up on this and figure out what we can do.

edmundito · 2022-11-09T14:55:38Z

airbyte-webapp/src/components/connection/CatalogTree/next/CatalogTreeTableRow.tsx

@@ -63,14 +63,13 @@ export const CatalogTreeTableRow: React.FC<StreamHeaderProps> = ({
    [styles.disabledChange]: changedSelected && !isStreamEnabled,
    [styles.selected]: isSelected,
    [styles.error]: hasError,
+    [styles.disabled]: !changedSelected && !isStreamEnabled,


I noticed that! I'm not a big fan, and maybe we can work around it by disabling it. Additionally, I feel the color contrast may be too low and we may need to revisit this with design.

edmundito · 2022-11-09T14:58:44Z

Also, the error state is a bit different for this new table but currently, it follows what the current table does:

Instead, the primary key/cursor dropdowns should highlight red. This is something we could address as a separate issue because the dropdowns still need a lot of work. How would you like to handle this?

teallarson · 2022-11-09T16:09:11Z

Also, the error state is a bit different for this new table but currently, it follows what the current table does:

Instead, the primary key/cursor dropdowns should highlight red. This is something we could address as a separate issue because the dropdowns still need a lot of work. How would you like to handle this?

what do you think about rolling that into #18243

teallarson · 2022-11-09T17:45:02Z

@edmundito addressed the comments you left above.

Something in this PR caused us to lose the ellipses for cell contents and made the text wrap instead. I got it to stop wrapping. Will look later this afternoon at getting the ellipses back the way they're supposed to be...

edmundito

Did another pass. Code changes look good but noticed a few details while testing (Chrome)

Looks like the table headings are not aligning with the columns.
I noticed that the sync mode dropdown does not stretch to the full size of the column.
The heading arrow can be offset from the columns

Here's a screenshot pointing all 3

There is a larger gap between the card heading and the search bar.

This is what's in the design:

And this is what I see:

teallarson · 2022-11-14T16:35:28Z

Did another pass. Code changes look good but noticed a few details while testing (Chrome)

Looks like the table headings are not aligning with the columns.

I noticed that the sync mode dropdown does not stretch to the full size of the column.

The heading arrow can be offset from the columns

Here's a screenshot pointing all 3

There is a larger gap between the card heading and the search bar.

This is what's in the design:

And this is what I see:

I addressed item #2. I propose that items 1 and 3 (heading alignment) be addressed in a separate PR. I've updated the description to say that and removed the "closes" verbage so the Issue stays open.

This reverts commit a72deb1.

This reverts commit ebbe5a9.

* 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]>

* typography * wip * continued progress * merge cleanup * table item alignment * row styling * remove extra item * continued styling * alignment works with scrollbar * headings align with or without scrollbars * stream connection header alignment * cleanup * pill-select justify * cleanup * fix ternary logic for disabled streams rows * streams table design tweaks * make table rows flush with card edge * review cleanup, fix scroll overflow for firefox * use gap not padding * add scrollbar-gutter to rows too * use ellipsis for cell text, not wrap * fix alignment of syncCatalogField * fix text ellipses and syncmode cell width * use a div to make this col static 200px * remove debug border * scss variables cleanup

This reverts commit a72deb1.

* 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]>

github-actions bot added area/platform issues related to the platform area/frontend Related to the Airbyte webapp labels Nov 3, 2022

teallarson force-pushed the teal/streams-table-design-pass branch from ce50396 to 44e25ef Compare November 4, 2022 13:28

teallarson commented Nov 5, 2022

View reviewed changes

teallarson marked this pull request as ready for review November 5, 2022 17:12

teallarson requested a review from a team as a code owner November 5, 2022 17:12

teallarson mentioned this pull request Nov 5, 2022

Update new connection streams table dropdowns #18243

Closed

teallarson commented Nov 5, 2022

View reviewed changes

teallarson changed the title ~~Streams table design pass~~ 🪟 🎨 Streams table design pass Nov 8, 2022

edmundito reviewed Nov 9, 2022

View reviewed changes

edmundito reviewed Nov 10, 2022

View reviewed changes

teallarson requested a review from edmundito November 10, 2022 20:31

teallarson and others added 14 commits November 10, 2022 15:41

typography

a7f09ef

wip

992e342

continued progress

d2affee

merge cleanup

a8e933d

table item alignment

2b180d1

row styling

4c6d248

remove extra item

004dbf9

continued styling

c655e4a

alignment works with scrollbar

c3f385e

headings align with or without scrollbars

bfd682b

stream connection header alignment

a6c8ffb

cleanup

4439cf0

pill-select justify

43a24d0

cleanup

b35b1e8

teallarson added 10 commits November 10, 2022 15:41

streams table design tweaks

9b2ca97

make table rows flush with card edge

3e86c5f

review cleanup, fix scroll overflow for firefox

9d13609

use gap not padding

81f46fd

add scrollbar-gutter to rows too

81a4b07

use ellipsis for cell text, not wrap

2094b52

fix alignment of syncCatalogField

e299741

fix text ellipses and syncmode cell width

07b68e4

use a div to make this col static 200px

d8f90bc

remove debug border

458914a

teallarson force-pushed the teal/streams-table-design-pass branch from 3a32476 to 458914a Compare November 10, 2022 20:41

teallarson and others added 2 commits November 10, 2022 16:11

scss variables cleanup

35d1808

Merge branch 'master' into teal/streams-table-design-pass

95a0186

teallarson mentioned this pull request Nov 14, 2022

New connection streams table design pass #17937

Closed

edmundito approved these changes Nov 14, 2022

View reviewed changes

teallarson merged commit a72deb1 into master Nov 14, 2022

teallarson deleted the teal/streams-table-design-pass branch November 14, 2022 17:54

teallarson mentioned this pull request Nov 14, 2022

🪟 🎨 New stream details panel #19219

Merged

teallarson added a commit that referenced this pull request Nov 14, 2022

Revert "🪟 🎨 Streams table design pass (#18908)"

721fec0

This reverts commit a72deb1.

teallarson mentioned this pull request Nov 14, 2022

Revert "🪟 🎨 Streams table design pass" #19397

Merged

teallarson added a commit that referenced this pull request Nov 14, 2022

Revert "🪟 🎨 Streams table design pass (#18908)" (#19397)

ebbe5a9

This reverts commit a72deb1.

teallarson restored the teal/streams-table-design-pass branch November 15, 2022 14:00

teallarson added a commit that referenced this pull request Nov 15, 2022

Revert "Revert "🪟 🎨 Streams table design pass (#18908)" (#19397)"

f8692d9

This reverts commit ebbe5a9.

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

akashkulk pushed a commit that referenced this pull request Dec 2, 2022

Revert "🪟 🎨 Streams table design pass (#18908)" (#19397)

1e53c32

This reverts commit a72deb1.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎨 Streams table design pass #18908

🪟 🎨 Streams table design pass #18908

teallarson commented Nov 3, 2022 •

edited

Loading

teallarson Nov 4, 2022

edmundito Nov 9, 2022

teallarson Nov 9, 2022

teallarson Nov 4, 2022

teallarson Nov 4, 2022

teallarson Nov 4, 2022

teallarson Nov 5, 2022

edmundito Nov 9, 2022

teallarson Nov 9, 2022

teallarson Nov 5, 2022

edmundito Nov 9, 2022

edmundito left a comment

edmundito Nov 9, 2022

teallarson Nov 9, 2022

edmundito Nov 9, 2022

edmundito Nov 9, 2022

edmundito Nov 9, 2022

edmundito Nov 9, 2022

edmundito commented Nov 9, 2022

teallarson commented Nov 9, 2022

teallarson commented Nov 9, 2022 •

edited

Loading

edmundito left a comment

teallarson commented Nov 14, 2022

🪟 🎨 Streams table design pass #18908

🪟 🎨 Streams table design pass #18908

Conversation

teallarson commented Nov 3, 2022 • edited Loading

What

How

Recommended reading order

Note:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmundito left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

edmundito commented Nov 9, 2022

teallarson commented Nov 9, 2022

teallarson commented Nov 9, 2022 • edited Loading

edmundito left a comment

Choose a reason for hiding this comment

teallarson commented Nov 14, 2022

teallarson commented Nov 3, 2022 •

edited

Loading

teallarson commented Nov 9, 2022 •

edited

Loading