🪟 🔧 Adds new Button component #16604

YatsukBogdan1 · 2022-09-12T17:53:08Z

What

How

I have refactored the existing Button component to match the new design, and also updated some component props according to Figma

YatsukBogdan1 · 2022-09-12T17:54:16Z

old PR #15720

krishnaglick · 2022-09-12T18:01:51Z

Can you reply to comments on the previous PR when you've addressed the issues?

YatsukBogdan1 · 2022-09-12T18:37:55Z

Can you reply to comments on the previous PR when you've addressed the issues?

done

airbyte-webapp/src/packages/cloud/views/auth/LoginPage/LoginPage.tsx

edmundito

Comments on the button props. I also noticed that the storybook is gone. Could you create a new storybook for the button?

edmundito · 2022-09-12T20:09:10Z

airbyte-webapp/src/components/base/Button/types.tsx

 export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
+  buttonRef?: MutableRefObject<HTMLButtonElement | null>;


forwardRef could be used to forward the button ref instead.

# Conflicts: # airbyte-webapp/src/packages/cloud/views/workspaces/WorkspacesPage/components/WorkspacesControl.tsx # airbyte-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/components/StatusView.tsx # airbyte-webapp/src/pages/OnboardingPage/components/VideoItem/components/ShowVideo.tsx

dizel852 · 2022-09-14T15:30:32Z

Tiny 🐛

dizel852 · 2022-09-14T16:05:26Z

airbyte-webapp/src/components/base/Button/index.stories.tsx

airbyte-webapp/src/components/base/Button/Button.tsx

airbyte-webapp/src/components/base/Input/Imput.module.scss

# Conflicts: # airbyte-webapp/src/components/base/Button/LoadingButton.tsx

…es to native button component; Fixes style in some places for button container; Removes unnecessary files

…states

# Conflicts: # airbyte-webapp/src/views/Feedback/SyncCompletedModal/components/ModalBody.tsx

timroes · 2022-09-14T13:03:57Z

airbyte-webapp/src/components/EntityTable/components/SortButton.module.scss

+  font-size: 11px;
+  line-height: 12px;
+  opacity: 0.4;
+  pointer-events: all;


❓ Why do we need the pointer-events: all here?

timroes · 2022-09-14T13:04:44Z

airbyte-webapp/src/components/EntityTable/components/SortButton.module.scss

+  align-items: center;
+  justify-content: center;
+  min-width: 18px;
+  font-size: 11px;


The button does not have text in it. What for are we setting the font-size value?

@timroes about SortButton (two comments below the same). earlier we used a base button component that only showed icon inside corresponding with sort order. still, for me, the better solution here is to make the table header clickable and get rid of the <Button /> component here. in this case, I don't need to make some magic with the base component, to make it look good, and also it will be easier for users to use sort, without the need to hit precisely small sorting button. what do you think?

timroes · 2022-09-14T13:05:22Z

airbyte-webapp/src/components/EntityTable/components/SortButton.module.scss

+  line-height: 12px;
+  opacity: 0.4;
+  pointer-events: all;
+  background: none !important;


❓ Is the !important here really needed? What style are we trying to overwrite with this?

airbyte-webapp/src/components/base/Button/Button.module.scss

dizel852 · 2022-09-15T10:01:55Z

One more tiny 🐛
Before changes:

After changes:

dizel852

Approved

edmundito

We're almost there... I did some testing and noted some issues that I found.

edmundito · 2022-09-22T19:26:08Z

airbyte-webapp/src/components/base/Button/Button.module.scss

+  }
+
+  &.typeSecondary {
+    background-color: colors.$white;


Secondary should have no background color assigned. In the cards it looks white, but outside of the card it takes the background color.

edmundito · 2022-09-22T19:30:37Z

airbyte-webapp/src/packages/cloud/views/credits/CreditsPage/components/RemainingCredits.tsx

-        </LoadingButton>
-        <Button as="a" target="_blank" href={config.links.contactSales}>
+        </Button>
+        <Button size="lg" onClick={() => window.open(config.links.contactSales, "_blank")}>


I'm not sure if the button is supposed to look larger than the other, but here's what I see:

edmundito · 2022-09-22T19:40:02Z

airbyte-webapp/src/components/ArrayOfObjectsEditor/components/EditorRow.tsx

        <Button
          type="button"
-          iconOnly
          arial-label={formatMessage({ id: "form.edit" })}
          onClick={() => onEdit(id)}
          disabled={disabled}
-        >
-          <PencilIcon />
-        </Button>
+          icon={<PencilIcon />}
+        />
        <Button
          type="button"
-          iconOnly
          aria-label={formatMessage({ id: "form.delete" })}
          onClick={() => onRemove(id)}
          disabled={disabled}
-        >
-          <CrossIcon />
-        </Button>
+          icon={<CrossIcon />}
+        />


These are missing a variant and they are too large:

This is what they should look like:

edmundito · 2022-09-22T19:56:33Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/UpgradeAllButton.tsx

+      <Button
+        className={styles.updateButton}
+        onClick={onUpdate}
+        isLoading={isLoading}
+        wasActive={hasSuccess}
+        icon={hasSuccess ? undefined : <TryArrow icon={faRedoAlt} />}
+      >
+        {hasSuccess ? <FormattedMessage id="admin.upgraded" /> : <FormattedMessage id="admin.upgradeAll" />}
+      </Button>


The buttons mismatch on this page:

edmundito · 2022-09-22T20:00:05Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/VersionCell.tsx

@@ -106,13 +106,13 @@ const VersionCell: React.FC<IProps> = ({ id, version, onChange, feedback, curren
                </InputField>
              )}
            </Field>
-            <LoadingButton
+            <Button


This button maybe needs to be smaller. Also this is what happens when I click on the button, it moves to the next line:

edmundito · 2022-09-22T20:03:18Z

airbyte-webapp/src/components/base/Button/types.tsx

+import React from "react";
+
+type ButtonSize = "xs" | "sm" | "lg";
+type ButtonVariant = "primary" | "secondary" | "danger" | "light" | "noStroke";


I understand that the name in the figma is no_stroke but maybe we can give it a better name. How about "clear" ?

# Conflicts: # airbyte-webapp/src/components/CreateConnectionContent/components/TryAfterErrorBlock.module.scss # airbyte-webapp/src/components/CreateConnectionContent/components/TryAfterErrorBlock.tsx # airbyte-webapp/src/components/EmptyResourceListView/EmptyResourceListView.tsx # airbyte-webapp/src/locales/en.json # airbyte-webapp/src/pages/OnboardingPage/components/ProgressBlock.tsx # airbyte-webapp/src/views/Connector/ServiceForm/components/FrequentlyUsedDestinations/__snapshots__/FrequentlyUsedDestinations.test.tsx.snap # airbyte-webapp/src/views/Connector/ServiceForm/components/StartWithDestination/__snapshots__/StartWithDestination.test.tsx.snap

edmundito

Noticed one thing while testing in the code and left a comment.

The styles for a couple of areas need to be adjusted:

The array of objects editor needs the paddings and button gaps updated to accommodate the new size of the buttons. I think the good thing about the button size is that they are easier to click so what needs to happen is to reduce the padding and the gap between them so they closely resemble the design.

Figma:
https://www.figma.com/file/etaOkOsnXMoKwbfhCD71kw/WEB-APP-%E2%9A%A0%EF%B8%8F%E2%9A%A0%EF%B8%8F-OLD-SCREENS---DO-NOT-IMPLEMENT-%E2%9A%A0%EF%B8%8F%E2%9A%A0%EF%B8%8F?node-id=34996%3A194795

The "Upgrade All" button is off by 1px. It looks like there is a div around it that may cause this to happen. Would it be possible to fix it?

edmundito · 2022-09-26T16:16:39Z

airbyte-webapp/src/pages/SettingsPage/pages/ConnectorsPage/components/VersionCell.tsx

-              type="submit"
-              disabled={(isSubmitting || !dirty) && !isConnectorUpdateable}
-            >
+            <Button size="xs" isLoading type="submit" disabled={(isSubmitting || !dirty) && !isConnectorUpdateable}>


Looks like the isLoading value is missing here so the button is always loading.

# Conflicts: # airbyte-webapp/src/components/EntityTable/components/StatusCell.tsx # airbyte-webapp/src/components/ui/Toast/SingletonCard.module.scss # airbyte-webapp/src/components/ui/Toast/Toast.tsx # airbyte-webapp/src/views/Connector/ServiceForm/components/FrequentlyUsedDestinations/__snapshots__/FrequentlyUsedDestinations.test.tsx.snap # airbyte-webapp/src/views/Connector/ServiceForm/components/StartWithDestination/__snapshots__/StartWithDestination.test.tsx.snap

…g property

edmundito

Tested both OSS and cloud and all buttons look good.

I did spot one more little issue with the gaps in one of the connector fields, but this can be fixed in a separate PR. See how the buttons don't have between the input or each other.

# Conflicts: # airbyte-webapp/src/packages/cloud/views/users/InviteUsersModal/InviteUsersModal.tsx # airbyte-webapp/src/packages/cloud/views/users/UsersSettingsView/UsersSettingsView.tsx # airbyte-webapp/src/views/Connection/CatalogDiffModal/CatalogDiffModal.tsx

* Add button component * Fixes comments on PR * Fixes style for old components * Fixes more style for old components * Update link usage of button * Fixes styles cascade * Fixes ref usage for buttons * Updates customStyles into className; Fixes error passing all properties to native button component; Fixes style in some places for button container; Removes unnecessary files * Fixes loading state for button; Adds storybook Button with different states * Fixes router issue; Fixes styles * Fixes SortButton; Fixes password eye button * Fixes comments * Update test snapshots * Fix plus icon; Fixes connection error button style * Fix buttons style; Updates variant noStroke -> clear * Fix version cell button * Update snapshots * Updates snapshots * Fixes settings button styles; Fixes editor row styles; Fixes isLoading property

YatsukBogdan1 added 4 commits September 12, 2022 20:21

Add button component

3202a69

Fixes comments on PR

fad2147

Fixes style for old components

96c1199

Fixes more style for old components

6ff11f4

YatsukBogdan1 requested a review from a team as a code owner September 12, 2022 17:53

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

YatsukBogdan1 mentioned this pull request Sep 12, 2022

[WIP] Adds new Button component #15720

Closed

YatsukBogdan1 added 2 commits September 12, 2022 21:27

Update link usage of button

2d2744d

Fixes styles cascade

134b6b6

dizel852 changed the title ~~Adds new Button component~~ 🪟 🔧 Adds new Button component Sep 12, 2022

krishnaglick reviewed Sep 12, 2022

View reviewed changes

airbyte-webapp/src/packages/cloud/views/auth/LoginPage/LoginPage.tsx Outdated Show resolved Hide resolved

edmundito reviewed Sep 12, 2022

View reviewed changes

YatsukBogdan1 added 2 commits September 14, 2022 13:19

Fixes ref usage for buttons

bcec4d4

dizel852 reviewed Sep 14, 2022

View reviewed changes

airbyte-webapp/src/components/base/Button/index.stories.tsx Outdated Show resolved Hide resolved

dizel852 reviewed Sep 14, 2022

View reviewed changes

airbyte-webapp/src/components/base/Button/Button.tsx Outdated Show resolved Hide resolved

dizel852 reviewed Sep 14, 2022

View reviewed changes

airbyte-webapp/src/components/base/Input/Imput.module.scss Outdated Show resolved Hide resolved

YatsukBogdan1 added 5 commits September 14, 2022 21:36

Merge branch 'master' into byatsuk/new-button-component

300af8b

# Conflicts: # airbyte-webapp/src/components/base/Button/LoadingButton.tsx

Updates customStyles into className; Fixes error passing all properti…

4cc9907

…es to native button component; Fixes style in some places for button container; Removes unnecessary files

Fixes loading state for button; Adds storybook Button with different …

4e8052e

…states

Merge branch 'master' into byatsuk/new-button-component

eec1849

Merge branch 'master' into byatsuk/new-button-component

c386063

# Conflicts: # airbyte-webapp/src/views/Feedback/SyncCompletedModal/components/ModalBody.tsx

timroes reviewed Sep 15, 2022

View reviewed changes

dizel852 requested review from edmundito and timroes September 22, 2022 14:48

dizel852 approved these changes Sep 22, 2022

View reviewed changes

edmundito mentioned this pull request Sep 22, 2022

Create common sortable column button #17062

Closed

edmundito suggested changes Sep 22, 2022

View reviewed changes

YatsukBogdan1 added 7 commits September 22, 2022 23:52

Merge branch 'master' into byatsuk/new-button-component

8fb47ed

Merge branch 'master' into byatsuk/new-button-component

162260f

Fix buttons style; Updates variant noStroke -> clear

88a60c5

Fix version cell button

2f391c5

Merge branch 'master' into byatsuk/new-button-component

b28e93d

Update snapshots

3a151a0

YatsukBogdan1 requested a review from edmundito September 26, 2022 09:15

YatsukBogdan1 added 2 commits September 26, 2022 12:43

Merge branch 'master' into byatsuk/new-button-component

4a9e595

Merge branch 'master' into byatsuk/new-button-component

4db1f3e

edmundito reviewed Sep 26, 2022

View reviewed changes

YatsukBogdan1 added 3 commits September 27, 2022 13:23

Updates snapshots

ddc07d9

Fixes settings button styles; Fixes editor row styles; Fixes isLoadin…

63428fb

…g property

edmundito approved these changes Sep 27, 2022

View reviewed changes

edmundito mentioned this pull request Sep 27, 2022

Make <Button /> component consistent #12705

Closed

YatsukBogdan1 merged commit ed8186e into master Sep 28, 2022

YatsukBogdan1 deleted the byatsuk/new-button-component branch September 28, 2022 11:01

octavia-squidington-iii mentioned this pull request Sep 28, 2022

Bump Airbyte version from 0.40.9 to 0.40.10 #17359

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🔧 Adds new Button component #16604

🪟 🔧 Adds new Button component #16604

YatsukBogdan1 commented Sep 12, 2022

YatsukBogdan1 commented Sep 12, 2022

krishnaglick commented Sep 12, 2022

YatsukBogdan1 commented Sep 12, 2022

edmundito left a comment

edmundito Sep 12, 2022

YatsukBogdan1 Sep 14, 2022

dizel852 commented Sep 14, 2022

dizel852 commented Sep 14, 2022

timroes Sep 14, 2022

timroes Sep 14, 2022

YatsukBogdan1 Sep 15, 2022

timroes Sep 14, 2022

dizel852 commented Sep 15, 2022

dizel852 left a comment

edmundito left a comment

edmundito Sep 22, 2022

edmundito Sep 22, 2022

edmundito Sep 22, 2022

edmundito Sep 22, 2022

edmundito Sep 22, 2022

edmundito Sep 22, 2022

edmundito left a comment

edmundito Sep 26, 2022

edmundito left a comment

		export interface ButtonProps extends React.ButtonHTMLAttributes<HTMLButtonElement> {
		buttonRef?: MutableRefObject<HTMLButtonElement \| null>;

🪟 🔧 Adds new Button component #16604

🪟 🔧 Adds new Button component #16604

Conversation

YatsukBogdan1 commented Sep 12, 2022

What

How

YatsukBogdan1 commented Sep 12, 2022

krishnaglick commented Sep 12, 2022

YatsukBogdan1 commented Sep 12, 2022

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

dizel852 commented Sep 14, 2022

dizel852 commented Sep 14, 2022

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

dizel852 commented Sep 15, 2022

dizel852 left a 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 left a 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