🪟 🎨 Updating design for BulkEditPanel component #18912

YatsukBogdan1 · 2022-11-03T14:54:18Z

What

Related to #18188

How

This PR continues the development of the BulkEditPanel component. Initial PR #18262
This PR implements the latest Figma design updates. Still working on hiding action buttons for connections form, and need to implement tests for this component

edmundito · 2022-11-08T00:53:57Z

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

@@ -28,7 +28,7 @@ export const Cell = styled.div<{
  lighter?: boolean;
  ellipsis?: boolean;
 }>`
-  flex: ${({ flex }) => flex || 1} 0 0;
+  flex: ${({ flex }) => (flex !== undefined ? flex : 1)} 0 0;


What was the reason behind this change?

when flex is passed as 0 it will be still 1 since 0 || 1 will result in 1, but my intention was to pass exactly 0

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

edmundito · 2022-11-08T01:07:36Z

airbyte-webapp/src/hooks/services/EditControls/BulkEditService.tsx

+
+const Context = React.createContext<EditControlsServiceContext | null>(null);
+
+export interface EditControlsServiceContext {


These should probably use the world Connection (for example, ConnectionEditControlsService), and the filename should also have this name over BulkEditService

actually, I found that both EditControls and BulkEditPanel both inside Formik, and also Formik have something called status object, where you can basically pass anything you want that helps you work with form
https://formik.org/docs/api/formik#status-any

A top-level status object that you can use to represent a form state that can't otherwise be expressed/stored with other methods. This is useful for capturing and passing through API responses to your inner component.

so I decided we can use it

edmundito · 2022-11-08T01:10:50Z

airbyte-webapp/src/hooks/services/BulkEdit/BulkEditService.tsx

@@ -57,6 +60,9 @@ export const BulkEditServiceProvider: React.FC<
  };

  const isActive = selectedBatchNodes.size > 0;
+  useEffect(() => {


This effect should be moved above line 43 so that it's below the existing hooks and above non-hooks.

edmundito · 2022-11-08T01:12:56Z

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

+  isActive: boolean;
+}
+
+const SchemaHeader = styled(Header)<SchemaHeaderProps>`


This still exists over SCSS because Header is a styled component?

edmundito · 2022-11-08T01:13:30Z

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

+
+.headerCell {
+  margin-right: variables.$spacing-xl;
+  padding-right: 0 !important;


These !important in the file are because of styled components?

edmundito · 2022-11-08T01:16:04Z

Looks like the unit tests are failing because of recent changes.

Tested locally, I noticed one more thing in regards to when there is no primary key or cursor. Looks like the design has changed to provide an "not available" state
https://www.figma.com/file/liLQhcbpVHiEDW18kiXQe3/01_03_CONNECTIONS?node-id=2732%3A60474

The field count doesn't have the same width as the design:

…ons to use Formik state object instead of EditControlsServiceContext; Moves useEffect inside BulkEditServiceProvider; Fixes design of streams count number

…anel in case if pkType or cursorType are null, to show StreamPathSelect in case these values are null

YatsukBogdan1 · 2022-11-09T21:33:49Z

@edmundito I updated BulkEditPanel to show StreamPathSelect in cases cursorType and pkType values are null, but we still need to update PillSelect styles, since according to Figma PillSelect with variant strong-blue and disabled: true should look like this:

currently, it looks like this in BulkEditPanel:

and what we need is this:

another case is when we have <source defined>:

should we have the same styles as when select is not available? just didn't find it in Figma

edmundito · 2022-11-10T14:52:43Z

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

 export const pathDisplayName = (path: Path): string => path.join(".");

 export type IndexerType = null | "required" | "sourceDefined";

 interface StreamPathSelectBaseProps {
  paths: Path[];
-  pathType: "required" | "sourceDefined";
+  pathType: "required" | "sourceDefined" | null;


Suggested change

pathType: "required" | "sourceDefined" | null;

pathType?: "required" | "sourceDefined";

edmundito

Changes look good. Discussed over Zoom that the state of the dropdowns will be updated separately. I left a couple of small notes, one should be fixed before merge.

edmundito · 2022-11-10T14:55:07Z

airbyte-webapp/src/hooks/services/BulkEdit/BulkEditService.tsx

  const [selectedBatchNodes, { reset, toggle, add }] = useSet<string | undefined>(new Set());
  const [options, setOptions] = useState<Partial<AirbyteStreamConfiguration>>(defaultOptions);

+  const isActive = selectedBatchNodes.size > 0;
+  useEffect(() => {
+    setStatus({ editControlsVisible: !isActive });


To be safe, the current status should be merge in case it's ever expanded.

setStatus({ ...status, editControlsVisible: !isActive });

…component

# Conflicts: # airbyte-webapp/src/components/SimpleTableComponents/SimpleTableComponents.tsx # airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss # airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx

…pe; Fixes Formik useStatus usage

# Conflicts: # airbyte-webapp/src/components/SimpleTableComponents/SimpleTableComponents.tsx # airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.module.scss # airbyte-webapp/src/components/connection/CatalogTree/next/StreamPathSelect.tsx

dizel852

Overall - LGTM 👍
Need to fix some comments before merge.

dizel852 · 2022-11-28T13:59:59Z

airbyte-webapp/src/components/connection/CatalogTree/next/BulkEditPanel.test.tsx

+const renderBulkEditPanel = async (): Promise<RenderResult> => {
+  let renderResult: RenderResult;
+  await act(async () => {
+    renderResult = render(
+      <IntlProvider locale="en" messages={en}>
+        <BulkEditPanel />
+      </IntlProvider>
+    );
+  });
+  return renderResult!;
+};


Can be simplified and doesn't throw any errors, so comment on the top can be removed

Suggested change

const renderBulkEditPanel = async (): Promise<RenderResult> => {

let renderResult: RenderResult;

await act(async () => {

renderResult = render(

<IntlProvider locale="en" messages={en}>

<BulkEditPanel />

</IntlProvider>

);

});

return renderResult!;

};

const renderBulkEditPanel = () =>

render(

<IntlProvider locale="en" messages={en}>

<BulkEditPanel />

</IntlProvider>

);

dizel852 · 2022-11-28T14:00:45Z

airbyte-webapp/src/components/connection/CatalogTree/next/BulkEditPanel.test.tsx

@@ -0,0 +1,230 @@
+/* eslint-disable @typescript-eslint/no-non-null-assertion */


can be removed after updating the renderBulkEditPanel function

dizel852 · 2022-11-28T14:01:16Z

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

+  isActive: boolean;
+}
+
+const SchemaHeader = styled(Header)<SchemaHeaderProps>`


# Conflicts: # airbyte-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/ConnectionReplicationTab.tsx

… of form buttons

Adds bulk edit panel new design

6820d92

YatsukBogdan1 requested a review from a team as a code owner November 3, 2022 14:54

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

YatsukBogdan1 marked this pull request as draft November 4, 2022 13:54

YatsukBogdan1 added 4 commits November 7, 2022 13:02

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

a3d8111

Adds: layout update for bulk edit; Applying styles for PillSelect

e860df1

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

99b18b6

Adds context for hiding and showing connection form action buttons

3a2dac8

edmundito reviewed Nov 8, 2022

View reviewed changes

YatsukBogdan1 marked this pull request as ready for review November 8, 2022 09:47

YatsukBogdan1 added 5 commits November 8, 2022 23:35

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

7204893

Fixes import string; Refactors logic of showing/hiding of action butt…

07cec68

…ons to use Formik state object instead of EditControlsServiceContext; Moves useEffect inside BulkEditServiceProvider; Fixes design of streams count number

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

fc11701

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

a42fc08

Adds disabled state for StreamPathSelect component; Updates BulkEditP…

a65c4b6

…anel in case if pkType or cursorType are null, to show StreamPathSelect in case these values are null

edmundito reviewed Nov 10, 2022

View reviewed changes

edmundito approved these changes Nov 10, 2022

View reviewed changes

YatsukBogdan1 added 10 commits November 13, 2022 21:08

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

209a12e

Fixes usage of Formik setStatus method; Fixes pathType of PathPopout …

8a23cf8

…component

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

abc15bd

Fixes style after for BulkEditPanel; Fixes pathType to type IndexerTy…

f77e174

…pe; Fixes Formik useStatus usage

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

d613878

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

696f43a

Add tests for BulkEditPanel

42fbf45

dizel852 approved these changes Nov 29, 2022

View reviewed changes

YatsukBogdan1 added 6 commits November 29, 2022 17:18

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

183da2b

Fix BulkEditPanel unit test

f44f2ff

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

f0dce06

# Conflicts: # airbyte-webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/ConnectionReplicationTab.tsx

Fix BulkEditServiceProvider useEffect hook for controlling visibility…

0212577

… of form buttons

Merge branch 'master' into byatsuk/feature-bulk-edit-panel-design-update

27d8328

Fix BulkEditServiceProvider useEffect hook for controlling visibility…

fbb2b32

… of form buttons

YatsukBogdan1 merged commit 975f1be into master Nov 30, 2022

YatsukBogdan1 deleted the byatsuk/feature-bulk-edit-panel-design-update branch November 30, 2022 20:49

github-actions bot mentioned this pull request Dec 2, 2022

Bump helm chart version reference to 0.42.4 #20029

Closed

This was referenced Dec 6, 2022

Bump Airbyte version from 0.40.23 to 0.40.24 #20161

Closed

Bump Airbyte version from 0.40.23 to 0.40.24 #20206

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🪟 🎨 Updating design for BulkEditPanel component #18912

🪟 🎨 Updating design for BulkEditPanel component #18912

YatsukBogdan1 commented Nov 3, 2022 •

edited by edmundito

Loading

edmundito Nov 8, 2022

YatsukBogdan1 Nov 8, 2022

edmundito Nov 8, 2022

YatsukBogdan1 Nov 8, 2022

edmundito Nov 8, 2022

edmundito Nov 8, 2022

YatsukBogdan1 Nov 8, 2022

dizel852 Nov 28, 2022

edmundito Nov 8, 2022

YatsukBogdan1 Nov 8, 2022

edmundito commented Nov 8, 2022 •

edited

Loading

YatsukBogdan1 commented Nov 9, 2022

edmundito Nov 10, 2022

edmundito left a comment

edmundito Nov 10, 2022

dizel852 left a comment

dizel852 Nov 28, 2022

dizel852 Nov 28, 2022

dizel852 Nov 28, 2022


		const Context = React.createContext<EditControlsServiceContext \| null>(null);

		export interface EditControlsServiceContext {

	pathType: "required" \| "sourceDefined" \| null;
	pathType?: "required" \| "sourceDefined";

		@@ -0,0 +1,230 @@
		/* eslint-disable @typescript-eslint/no-non-null-assertion */

🪟 🎨 Updating design for BulkEditPanel component #18912

🪟 🎨 Updating design for BulkEditPanel component #18912

Conversation

YatsukBogdan1 commented Nov 3, 2022 • edited by edmundito Loading

What

How

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 commented Nov 8, 2022 • edited Loading

YatsukBogdan1 commented Nov 9, 2022

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

dizel852 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

YatsukBogdan1 commented Nov 3, 2022 •

edited by edmundito

Loading

edmundito commented Nov 8, 2022 •

edited

Loading