Refactor & convert `PageTitle` to SCSS #17139

natalyjazzviolin · 2022-09-26T12:26:28Z

What

Closes #1569

How

Converts the PageTitle from styled components to SCSS.
This should not cause any UI changes. Example of a PageTitle component is below (Destinations & New Destination button):

Recommended reading order

airbyte-webapp/src/components/PageTitle/PageTitle.tsx
airbyte-webapp/src/components/PageTitle/PageTitle.module.scss

krishnaglick

The Connection string is smaller now. I think fixing my comment may remedy this.

krishnaglick · 2022-09-26T14:07:45Z

package-lock.json

+  "lockfileVersion": 2,
+  "requires": true,
+  "packages": {}
+}


This looks to have been generated in error and should be removed.

krishnaglick · 2022-09-26T14:12:22Z

airbyte-webapp/src/components/PageTitle/PageTitle.tsx

-  <MainContainer withLine={withLine}>
-    <TitleBlock>{title}</TitleBlock>
+  <div className={classNames(styles.container)} data-withLine={withLine}>
+    <H3


This H3 is deprecated.
/** @deprecated Use `<Text as="h1 | h2 | h3 | h4 | h5" size="md" />` */

edmundito · 2022-09-26T17:23:49Z

airbyte-webapp/src/components/PageTitle/PageTitle.tsx

    {middleTitleBlock ? (
-      <MiddleTitleBlock>{middleTitleBlock}</MiddleTitleBlock>
+      <Text as="h3" size="md" className={classNames(styles.middleTitleBlock)}>


Because this is the page title, we should use "h1" instead of "h3"

edmundito · 2022-09-26T17:24:28Z

airbyte-webapp/src/components/PageTitle/PageTitle.tsx

-  overflow: hidden;
-  text-overflow: ellipsis;
-`;
-
 const PageTitle: React.FC<PageTitleProps> = ({ title, withLine, middleComponent, middleTitleBlock, endComponent }) => (


This component should be named PageHeader and moved to src/components/ui/
Can you also add a storybook file?

@edmundito added a storybook file with a Primary story. I wasn't sure of the best way to implement the 'middle' component in a story as it needs to hold state, if that needs to be included in this PR - happy to do it, but need more direction :)

edmundito · 2022-09-27T14:31:22Z

airbyte-webapp/src/components/ui/PageHeader/index.tsx

+import PageHeader from "./PageHeader";
+
+export default PageHeader;
+export { PageHeader };


Suggested change

import PageHeader from "./PageHeader";

export default PageHeader;

export { PageHeader };

export * from "./PageHeader";

edmundito · 2022-09-27T14:31:31Z

airbyte-webapp/src/components/ui/PageHeader/PageHeader.tsx

+  title: React.ReactNode;
+}
+
+const PageHeader: React.FC<PageHeaderProps> = ({


Suggested change

const PageHeader: React.FC<PageHeaderProps> = ({

export const PageHeader: React.FC<PageHeaderProps> = ({

edmundito · 2022-09-27T14:31:48Z

airbyte-webapp/src/components/ui/PageHeader/PageHeader.tsx

+  </div>
+);
+
+export default PageHeader;


The codebase is moving away from using the default export

Suggested change

export default PageHeader;

natalyjazzviolin · 2022-09-30T18:11:28Z

@krishnaglick I compared just to make sure, looks the same to me at this point:

Cloud:

This branch:

krishnaglick

One last comment then LGTM

krishnaglick · 2022-09-30T19:30:34Z

airbyte-webapp/src/components/ui/PageHeader/index.tsx

+import { PageHeader } from "./PageHeader";
+
+export default PageHeader;
+export { PageHeader };


Suggested change

import { PageHeader } from "./PageHeader";

export default PageHeader;

export { PageHeader };

export * from "./PageHeader";

…ations * master: (75 commits) source-sentry: migrate to per-stream states (#17466) Greg/clickhouse polishing (#17483) upgrade debezium version to 1.9.6 (#17459) 🐛 Source Twilio: Lookback_window config (#17478) hide S3 source connector from catalog (#17472) 🪟 Migrate styles for Connection-related Components (#17339) Added new title (#17480) Refactor & convert `PageTitle` to SCSS (#17139) updated releaseStage for zendesk-talk (#17477) [low-code] Apply log level to stream loggers (#17284) 🐛 Source Salesforce: filter out objects not supported by the Bulk API (#17453) Source Marketo: certify GA (#17445) Update greenhouse paginator (#17429) Add some services start validation to acceptance_tests.sh (#17425) 📖 Removes $ from terminal commands to allow direct copying. (#17467) migrate source GA connectors to per-stream states (2) (#17410) Source Klaviyo: bump CDK dependency (#17422) Source Pinterest: change releaseStage to GA (#17045) Source Pinterest: Set start_date dynamically based on API restrictions for lookup (#17387) updated releaseStage to generally_available (#17374) ...

* master: source-sentry: migrate to per-stream states (#17466) Greg/clickhouse polishing (#17483) upgrade debezium version to 1.9.6 (#17459) 🐛 Source Twilio: Lookback_window config (#17478) hide S3 source connector from catalog (#17472) 🪟 Migrate styles for Connection-related Components (#17339) Added new title (#17480) Refactor & convert `PageTitle` to SCSS (#17139) updated releaseStage for zendesk-talk (#17477) [low-code] Apply log level to stream loggers (#17284) 🐛 Source Salesforce: filter out objects not supported by the Bulk API (#17453) Source Marketo: certify GA (#17445) Update greenhouse paginator (#17429) Add some services start validation to acceptance_tests.sh (#17425) 📖 Removes $ from terminal commands to allow direct copying. (#17467)

* Converts PageTitle from styled components to SCSS. * Delete package-lock.json * Swapped deprecated H3 for Text base component. * Renames and moves the PageTitle component. * Adds primary story. * Removes PageTitle component. * Regenerates failing snapshot tests. * Requested changes.

Converts PageTitle from styled components to SCSS.

e883c58

natalyjazzviolin requested a review from a team as a code owner September 26, 2022 12:26

natalyjazzviolin marked this pull request as draft September 26, 2022 12:26

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

natalyjazzviolin force-pushed the nataly-update-pageTitle branch from 9191f02 to e883c58 Compare September 26, 2022 13:49

Merge branch 'master' into nataly-update-pageTitle

5d81eed

natalyjazzviolin marked this pull request as ready for review September 26, 2022 14:06

natalyjazzviolin changed the title ~~Refactor & convertPageTitle to SCSS~~ Refactor & convert PageTitle to SCSS Sep 26, 2022

krishnaglick reviewed Sep 26, 2022

View reviewed changes

natalyjazzviolin and others added 2 commits September 26, 2022 11:00

Delete package-lock.json

ef00784

Swapped deprecated H3 for Text base component.

030a8ba

natalyjazzviolin requested a review from edmundito September 26, 2022 15:08

edmundito reviewed Sep 26, 2022

View reviewed changes

natalyjazzviolin and others added 3 commits September 26, 2022 14:12

Merge branch 'master' into nataly-update-pageTitle

e47a230

Merge branch 'master' into nataly-update-pageTitle

f5aaac9

Renames and moves the PageTitle component.

d4c67ca

edmundito reviewed Sep 27, 2022

View reviewed changes

natalyjazzviolin marked this pull request as draft September 29, 2022 20:03

nataly added 3 commits September 29, 2022 16:17

Resolving conflicts.

f5abce8

Adds primary story.

4e29450

Removes PageTitle component.

b98fab3

Merge branch 'master' into nataly-update-pageTitle

7f2a67a

natalyjazzviolin marked this pull request as ready for review September 30, 2022 18:11

natalyjazzviolin requested review from krishnaglick and edmundito September 30, 2022 18:12

krishnaglick reviewed Sep 30, 2022

View reviewed changes

natalyjazzviolin and others added 2 commits September 30, 2022 15:44

Merge branch 'master' into nataly-update-pageTitle

1046589

Regenerates failing snapshot tests.

21c1ab3

Requested changes.

ebe4674

krishnaglick approved these changes Sep 30, 2022

View reviewed changes

natalyjazzviolin merged commit 0daa9f2 into master Sep 30, 2022

natalyjazzviolin deleted the nataly-update-pageTitle branch September 30, 2022 20:16

octavia-squidington-iii mentioned this pull request Oct 5, 2022

Bump Airbyte version from 0.40.10 to 0.40.11 #17610

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor & convert `PageTitle` to SCSS #17139

Refactor & convert `PageTitle` to SCSS #17139

natalyjazzviolin commented Sep 26, 2022 •

edited

Loading

krishnaglick left a comment

krishnaglick Sep 26, 2022

natalyjazzviolin Sep 26, 2022

krishnaglick Sep 26, 2022 •

edited

Loading

edmundito Sep 26, 2022

edmundito Sep 26, 2022

natalyjazzviolin Sep 30, 2022

edmundito Sep 27, 2022

edmundito Sep 27, 2022

edmundito Sep 27, 2022

natalyjazzviolin commented Sep 30, 2022

krishnaglick left a comment

krishnaglick Sep 30, 2022

	const PageHeader: React.FC<PageHeaderProps> = ({
	export const PageHeader: React.FC<PageHeaderProps> = ({

Refactor & convert PageTitle to SCSS #17139

Refactor & convert PageTitle to SCSS #17139

Conversation

natalyjazzviolin commented Sep 26, 2022 • edited Loading

What

How

Recommended reading order

krishnaglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

krishnaglick Sep 26, 2022 • edited Loading

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

natalyjazzviolin commented Sep 30, 2022

krishnaglick left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Refactor & convert `PageTitle` to SCSS #17139

Refactor & convert `PageTitle` to SCSS #17139

natalyjazzviolin commented Sep 26, 2022 •

edited

Loading

krishnaglick Sep 26, 2022 •

edited

Loading