-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
* Add useAdvancedModeSetting hook * Add advanced mode toggle to workspace settings As of this commit, it's a bit ugly since the toggle switch has no margin between it and the text input directly above; going to migrate the page to use scss modules instead of styling it by the current page conventions. * Query the current workspace inside useAdvancedMode This setting is only relevant to single-workspace views; this simplifies client code. * Style workspace settings form with scss module The switch needed a top-margin, too, so I created an scss module to hold that style. Also migrated the component-local `Header` and `Buttons` styled-components. * Only show connection state if advanced mode is enabled * Don't call setAdvancedMode until form is submitted * Add external label, extract text to i18n keys Keeping the className-passing behavior in LabeledSwitch, though, it seems to me to be a strictly better API for that component. * Add tooltip to advanced mode switch * Add unit test for useAdvancedModeSetting hook * Add unit test for the SettingsView component * Address PR comments - clean up object destructuring/composition - expand inter-item spacing from 10px to 21px to match related form * Add advanced mode toggle to OSS account settings * Remove commented-out button container component * mock useTrackPage since it was added to SettingsView Co-authored-by: lmossman <lake@airbyte.io>
- Loading branch information
1 parent
e8bc3d7
commit 68ab523
Showing
12 changed files
with
288 additions
and
52 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
53 changes: 53 additions & 0 deletions
53
airbyte-webapp/src/hooks/services/useAdvancedModeSetting.test.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| import { act, renderHook } from "@testing-library/react-hooks"; | ||
|
|
||
| import { useAdvancedModeSetting } from "./useAdvancedModeSetting"; | ||
|
|
||
| // mock `useCurrentWorkspace` with a closure so we can simulate changing | ||
| // workspaces by mutating the top-level variable it references | ||
| let mockWorkspaceId = "fakeWorkspaceId"; | ||
| const changeToWorkspace = (newWorkspaceId: string) => { | ||
| mockWorkspaceId = newWorkspaceId; | ||
| }; | ||
|
|
||
| jest.mock("hooks/services/useWorkspace", () => ({ | ||
| useCurrentWorkspace() { | ||
| return { workspaceId: mockWorkspaceId }; | ||
| }, | ||
| })); | ||
|
|
||
| test("it defaults to false before advanced mode is explicitly set", () => { | ||
| const { result } = renderHook(() => useAdvancedModeSetting()); | ||
| // eslint-disable-next-line prefer-const | ||
| let [isAdvancedMode, setAdvancedMode] = result.current; | ||
|
|
||
| expect(isAdvancedMode).toBe(false); | ||
|
|
||
| act(() => setAdvancedMode(true)); | ||
| [isAdvancedMode] = result.current; | ||
|
|
||
| expect(isAdvancedMode).toBe(true); | ||
| }); | ||
|
|
||
| test("it stores workspace-specific advanced mode settings", () => { | ||
| changeToWorkspace("workspaceA"); | ||
|
|
||
| const { result, rerender } = renderHook(() => useAdvancedModeSetting()); | ||
| // Avoiding destructuring in this spec to avoid capturing stale values when | ||
| // rerendering in different workspaces | ||
| const setAdvancedModeA = result.current[1]; | ||
|
|
||
| expect(result.current[0]).toBe(false); | ||
| act(() => setAdvancedModeA(true)); | ||
|
|
||
| expect(result.current[0]).toBe(true); | ||
|
|
||
| // in workspaceB, it returns the default setting of `false` | ||
| changeToWorkspace("workspaceB"); | ||
| rerender(); | ||
| expect(result.current[0]).toBe(false); | ||
|
|
||
| // ...but workspaceA's manual setting is persisted | ||
| changeToWorkspace("workspaceA"); | ||
| rerender(); | ||
| expect(result.current[0]).toBe(true); | ||
| }); |
19 changes: 19 additions & 0 deletions
19
airbyte-webapp/src/hooks/services/useAdvancedModeSetting.ts
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { useLocalStorage } from "react-use"; | ||
|
|
||
| import { useCurrentWorkspace } from "hooks/services/useWorkspace"; | ||
|
|
||
| type SettingsByWorkspace = Record<string, boolean>; | ||
|
|
||
| export const useAdvancedModeSetting = (): [boolean, (newSetting: boolean) => void] => { | ||
| const { workspaceId } = useCurrentWorkspace(); | ||
| const [advancedModeSettingsByWorkspace, setAdvancedModeSettingsByWorkspace] = useLocalStorage<SettingsByWorkspace>( | ||
| "advancedMode", | ||
| {} | ||
| ); | ||
|
|
||
| const isAdvancedMode = (advancedModeSettingsByWorkspace || {})[workspaceId] ?? false; | ||
| const setAdvancedMode = (newSetting: boolean) => | ||
| setAdvancedModeSettingsByWorkspace({ ...advancedModeSettingsByWorkspace, [workspaceId]: newSetting }); | ||
|
|
||
| return [isAdvancedMode, setAdvancedMode]; | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
20 changes: 20 additions & 0 deletions
20
...c/packages/cloud/views/workspaces/WorkspaceSettingsView/WorkspaceSettingsView.module.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| .formItem { | ||
| margin-top: 21px; | ||
| width: 100%; | ||
| } | ||
|
|
||
| .buttonGroup { | ||
| display: flex; | ||
| flex-direction: row; | ||
| justify-content: flex-end; | ||
| align-items: center; | ||
|
|
||
| & > button { | ||
| margin-left: 5px; | ||
| } | ||
| } | ||
|
|
||
| .header { | ||
| display: flex; | ||
| justify-content: space-between; | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
48 changes: 48 additions & 0 deletions
48
...webapp/src/pages/ConnectionPage/pages/ConnectionItemPage/components/SettingsView.test.tsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,48 @@ | ||
| import { render, mockConnection } from "utils/testutils"; | ||
|
|
||
| import SettingsView from "./SettingsView"; | ||
|
|
||
| let mockIsAdvancedMode = false; | ||
| const setMockIsAdvancedMode = (newSetting: boolean) => { | ||
| mockIsAdvancedMode = newSetting; | ||
| }; | ||
| jest.mock("hooks/services/useAdvancedModeSetting", () => ({ | ||
| useAdvancedModeSetting() { | ||
| return [mockIsAdvancedMode, setMockIsAdvancedMode]; | ||
| }, | ||
| })); | ||
|
|
||
| jest.mock("hooks/services/useConnectionHook", () => ({ | ||
| useDeleteConnection: () => ({ mutateAsync: () => null }), | ||
| useGetConnectionState: () => ({ state: null, globalState: null, streamState: null }), | ||
| })); | ||
|
|
||
| jest.mock("hooks/services/Analytics/useAnalyticsService", () => ({ | ||
| useTrackPage: () => null, | ||
| })); | ||
|
|
||
| // Mocking the DeleteBlock component is a bit ugly, but it's simpler and less | ||
| // brittle than mocking the providers it depends on; at least it's a direct, | ||
| // visible dependency of the component under test here. | ||
| // | ||
| // This mock is intentionally trivial; if anything to do with this component is | ||
| // to be tested, we'll have to bite the bullet and render it properly, within | ||
| // the necessary providers. | ||
| jest.mock("components/DeleteBlock", () => () => { | ||
| const MockDeleteBlock = () => <div>Does not actually delete anything</div>; | ||
| return <MockDeleteBlock />; | ||
| }); | ||
|
|
||
| describe("<SettingsView />", () => { | ||
| test("it only renders connection state when advanced mode is enabled", async () => { | ||
| let container: HTMLElement; | ||
|
|
||
| setMockIsAdvancedMode(false); | ||
| ({ container } = await render(<SettingsView connection={mockConnection} />)); | ||
| expect(container.textContent).not.toContain("Connection State"); | ||
|
|
||
| setMockIsAdvancedMode(true); | ||
| ({ container } = await render(<SettingsView connection={mockConnection} />)); | ||
| expect(container.textContent).toContain("Connection State"); | ||
| }); | ||
| }); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7 changes: 7 additions & 0 deletions
7
airbyte-webapp/src/pages/SettingsPage/pages/AccountPage/components/AccountForm.module.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| .formItem { | ||
| margin-bottom: 21px; | ||
| } | ||
|
|
||
| .submit { | ||
| margin-bottom: 10px; | ||
| } |
Oops, something went wrong.