From 204ac9d645a412e9b2c6dee63ff1e130c0047bd8 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 6 Jun 2024 17:04:45 +0800 Subject: [PATCH 1/9] add patch style for oui fixed components Signed-off-by: tygao --- public/chat_header_button.tsx | 2 + public/hooks/use_patch_fixed_style.ts | 89 +++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 public/hooks/use_patch_fixed_style.ts diff --git a/public/chat_header_button.tsx b/public/chat_header_button.tsx index 1c8240de..9723010a 100644 --- a/public/chat_header_button.tsx +++ b/public/chat_header_button.tsx @@ -24,6 +24,7 @@ import { } from './utils/constants'; import { useCore } from './contexts/core_context'; import { MountPointPortal } from '../../../src/plugins/opensearch_dashboards_react/public'; +import { usePatchFixedStyle } from './hooks/use_patch_fixed_style'; interface HeaderChatButtonProps { application: ApplicationStart; @@ -52,6 +53,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { const core = useCore(); const flyoutFullScreen = sidecarDockedMode === SIDECAR_DOCKED_MODE.TAKEOVER; const flyoutMountPoint = useRef(null); + usePatchFixedStyle(); useEffectOnce(() => { const subscription = props.application.currentAppId$.subscribe((id) => setAppId(id)); diff --git a/public/hooks/use_patch_fixed_style.ts b/public/hooks/use_patch_fixed_style.ts new file mode 100644 index 00000000..dc240853 --- /dev/null +++ b/public/hooks/use_patch_fixed_style.ts @@ -0,0 +1,89 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ +import { useEffect, useRef } from 'react'; + +import { useCore } from '../contexts/core_context'; +import { ISidecarConfig, SIDECAR_DOCKED_MODE } from '../../../../src/core/public'; + +// There are some UI components from library whose position is fixed and are not compatible with each other and also not compatible with sidecar container. +// There is currently no way to provide config for these components at runtime. +// This hook patches a style for all these already known components to make them compatible with sidecar. +// TODO: Use config provider from UI library to make this more reasonable. +export const usePatchFixedStyle = () => { + const core = useCore(); + const sidecarConfig$ = core.overlays.sidecar().getSidecarConfig$(); + const textRef = useRef(); + + useEffect(() => { + const css = ''; + const style = document.createElement('style'); + const text = document.createTextNode(css); + document.head.appendChild(style); + style.appendChild(text); + + textRef.current = text; + + const subscription = sidecarConfig$.subscribe((config) => { + if (!config) return; + updateHeadStyle(config, textRef.current); + }); + return () => { + subscription.unsubscribe(); + }; + }, []); +}; + +function updateHeadStyle(config: ISidecarConfig, text?: Text) { + let css = ''; + if (!text) return; + + if ( + // When sidecar is opened and docked position is left or right, we should patch style. + config?.isHidden !== true && + config.paddingSize && + (config.dockedMode === SIDECAR_DOCKED_MODE.LEFT || + config.dockedMode === SIDECAR_DOCKED_MODE.RIGHT) + ) { + const { dockedMode, paddingSize } = config; + if (dockedMode === SIDECAR_DOCKED_MODE.RIGHT) { + // Current applied components include flyout and bottomBar. + // Although the class names of actual rendered component start with eui. We will also apply oui for fallback. + css = ` + .euiFlyout:not(.euiFlyout--left) { + right: ${paddingSize}px + } + .ouiFlyout:not(.ouiFlyout--left) { + right: ${paddingSize}px + } + .euiBottomBar{ + padding-right: ${paddingSize}px + } + .ouiBottomBar{ + padding-right: ${paddingSize}px + } + `; + } else if (dockedMode === SIDECAR_DOCKED_MODE.LEFT) { + css = ` + .euiFlyout--left { + left: ${paddingSize}px + } + .ouiFlyout--left { + left: ${paddingSize}px + } + .euiBottomBar{ + padding-left: ${paddingSize}px + } + .ouiBottomBar{ + padding-left: ${paddingSize}px + } + `; + } + } + + // If sidecar closes or docked direction change to takeover mode, we will keep css empty to remove patch style and update. + requestAnimationFrame(() => { + text.textContent = css; + }); +} From 632adbf20455dcee5257a39457a921a70bf553b6 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 6 Jun 2024 17:21:09 +0800 Subject: [PATCH 2/9] add remove child Signed-off-by: tygao --- public/hooks/use_patch_fixed_style.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/public/hooks/use_patch_fixed_style.ts b/public/hooks/use_patch_fixed_style.ts index dc240853..db9ba953 100644 --- a/public/hooks/use_patch_fixed_style.ts +++ b/public/hooks/use_patch_fixed_style.ts @@ -31,8 +31,12 @@ export const usePatchFixedStyle = () => { }); return () => { subscription.unsubscribe(); + document.head.removeChild(style); + if (textRef.current) { + style.removeChild(textRef.current); + } }; - }, []); + }, [sidecarConfig$]); }; function updateHeadStyle(config: ISidecarConfig, text?: Text) { From df0d55f3db1e0ea6b610f7b2148fe6105870ca45 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 6 Jun 2024 17:22:44 +0800 Subject: [PATCH 3/9] doc: update changelog Signed-off-by: tygao --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8ad35e..61c07477 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,4 +16,5 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) - Add data source service ([#191](https://github.com/opensearch-project/dashboards-assistant/pull/191)) - Update router and controller to support MDS ([#190](https://github.com/opensearch-project/dashboards-assistant/pull/190)) - Hide notebook feature when MDS enabled and remove security dashboard plugin dependency ([#201](https://github.com/opensearch-project/dashboards-assistant/pull/201)) -- Refactor default data source retriever ([#197](https://github.com/opensearch-project/dashboards-assistant/pull/197)) \ No newline at end of file +- Refactor default data source retriever ([#197](https://github.com/opensearch-project/dashboards-assistant/pull/197)) +- Add patch style for fixed components ([#203](https://github.com/opensearch-project/dashboards-assistant/pull/203)) From d097bf5ed7e5acf9585e5535462331c5ed89b927 Mon Sep 17 00:00:00 2001 From: tygao Date: Thu, 6 Jun 2024 17:59:58 +0800 Subject: [PATCH 4/9] update test mock Signed-off-by: tygao --- public/chat_header_button.test.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/public/chat_header_button.test.tsx b/public/chat_header_button.test.tsx index f6ebae40..a26f5fbd 100644 --- a/public/chat_header_button.test.tsx +++ b/public/chat_header_button.test.tsx @@ -74,6 +74,9 @@ jest.spyOn(coreContextExports, 'useCore').mockReturnValue({ element.style.display = 'block'; } }, + getSidecarConfig$: () => { + return new BehaviorSubject(undefined); + }, }; }, }, From 6b778be3235dde7468b512dec439b2de53160b03 Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 7 Jun 2024 14:04:46 +0800 Subject: [PATCH 5/9] add test for hook Signed-off-by: tygao --- .../use_patch_fixed_style.test.ts.snap | 119 ++++++++++++++++++ public/hooks/use_patch_fixed_style.test.ts | 90 +++++++++++++ 2 files changed, 209 insertions(+) create mode 100644 public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap create mode 100644 public/hooks/use_patch_fixed_style.test.ts diff --git a/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap b/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap new file mode 100644 index 00000000..fab4db5c --- /dev/null +++ b/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap @@ -0,0 +1,119 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`usePatchFixedStyle hook should patch corresponding left style when sidecarConfig$ pipe 1`] = ` + + + + + + +`; + +exports[`usePatchFixedStyle hook should patch corresponding right style when sidecarConfig$ pipe 1`] = ` + + + + + + +`; + +exports[`usePatchFixedStyle hook should patch empty style when dockedMode of sidecarConfig$ is takeover 1`] = ` + + + + + + +`; + +exports[`usePatchFixedStyle hook should patch empty style when isHidden of sidecarConfig$ is true 1`] = ` + + + + + + +`; diff --git a/public/hooks/use_patch_fixed_style.test.ts b/public/hooks/use_patch_fixed_style.test.ts new file mode 100644 index 00000000..81b3a31d --- /dev/null +++ b/public/hooks/use_patch_fixed_style.test.ts @@ -0,0 +1,90 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +import { renderHook, act } from '@testing-library/react-hooks'; + +import { usePatchFixedStyle } from './use_patch_fixed_style'; +import * as coreHookExports from '../contexts/core_context'; +import { BehaviorSubject } from 'rxjs'; +import { ISidecarConfig, SIDECAR_DOCKED_MODE } from '../../../../src/core/public'; + +describe('usePatchFixedStyle hook', () => { + const sidecarConfig$ = new BehaviorSubject({ + dockedMode: SIDECAR_DOCKED_MODE.RIGHT, + paddingSize: 300, + }); + + beforeEach(() => { + jest.spyOn(coreHookExports, 'useCore').mockReturnValue({ + overlays: { + // @ts-ignore + sidecar: () => { + return { + getSidecarConfig$: () => { + return sidecarConfig$; + }, + }; + }, + }, + }); + }); + + afterEach(() => { + jest.clearAllMocks(); + }); + + it('should patch corresponding left style when sidecarConfig$ pipe', async () => { + renderHook(() => usePatchFixedStyle()); + act(() => + sidecarConfig$.next({ + dockedMode: SIDECAR_DOCKED_MODE.LEFT, + paddingSize: 300, + }) + ); + await new Promise((r) => setTimeout(r, 2000)); + + expect(document.head).toMatchSnapshot(); + }); + + it('should patch corresponding right style when sidecarConfig$ pipe', async () => { + renderHook(() => usePatchFixedStyle()); + act(() => + sidecarConfig$.next({ + dockedMode: SIDECAR_DOCKED_MODE.RIGHT, + paddingSize: 300, + }) + ); + await new Promise((r) => setTimeout(r, 2000)); + + expect(document.head).toMatchSnapshot(); + }); + + it('should patch empty style when isHidden of sidecarConfig$ is true', async () => { + renderHook(() => usePatchFixedStyle()); + act(() => + sidecarConfig$.next({ + dockedMode: SIDECAR_DOCKED_MODE.LEFT, + paddingSize: 300, + isHidden: true, + }) + ); + await new Promise((r) => setTimeout(r, 2000)); + + expect(document.head).toMatchSnapshot(); + }); + + it('should patch empty style when dockedMode of sidecarConfig$ is takeover', async () => { + renderHook(() => usePatchFixedStyle()); + act(() => + sidecarConfig$.next({ + dockedMode: SIDECAR_DOCKED_MODE.TAKEOVER, + paddingSize: 300, + }) + ); + await new Promise((r) => setTimeout(r, 2000)); + + expect(document.head).toMatchSnapshot(); + }); +}); From 7ac58cfd9a321aa2e77c05328965e6cbaf540d6b Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 7 Jun 2024 14:13:05 +0800 Subject: [PATCH 6/9] remove useRef Signed-off-by: tygao --- public/hooks/use_patch_fixed_style.ts | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/public/hooks/use_patch_fixed_style.ts b/public/hooks/use_patch_fixed_style.ts index db9ba953..66d8ca30 100644 --- a/public/hooks/use_patch_fixed_style.ts +++ b/public/hooks/use_patch_fixed_style.ts @@ -14,7 +14,6 @@ import { ISidecarConfig, SIDECAR_DOCKED_MODE } from '../../../../src/core/public export const usePatchFixedStyle = () => { const core = useCore(); const sidecarConfig$ = core.overlays.sidecar().getSidecarConfig$(); - const textRef = useRef(); useEffect(() => { const css = ''; @@ -23,18 +22,14 @@ export const usePatchFixedStyle = () => { document.head.appendChild(style); style.appendChild(text); - textRef.current = text; - const subscription = sidecarConfig$.subscribe((config) => { if (!config) return; - updateHeadStyle(config, textRef.current); + updateHeadStyle(config, text); }); return () => { subscription.unsubscribe(); document.head.removeChild(style); - if (textRef.current) { - style.removeChild(textRef.current); - } + style.removeChild(text); }; }, [sidecarConfig$]); }; From 40ea8ed02f97252cefc096b07717d268859dec06 Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 7 Jun 2024 15:08:14 +0800 Subject: [PATCH 7/9] use mock requestAnimationFrame to replace delay Signed-off-by: tygao --- public/hooks/use_patch_fixed_style.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/public/hooks/use_patch_fixed_style.test.ts b/public/hooks/use_patch_fixed_style.test.ts index 81b3a31d..981c4d38 100644 --- a/public/hooks/use_patch_fixed_style.test.ts +++ b/public/hooks/use_patch_fixed_style.test.ts @@ -29,10 +29,12 @@ describe('usePatchFixedStyle hook', () => { }, }, }); + jest.spyOn(window, 'requestAnimationFrame').mockImplementation((cb) => cb()); }); afterEach(() => { jest.clearAllMocks(); + window.requestAnimationFrame.mockRestore(); }); it('should patch corresponding left style when sidecarConfig$ pipe', async () => { @@ -43,7 +45,6 @@ describe('usePatchFixedStyle hook', () => { paddingSize: 300, }) ); - await new Promise((r) => setTimeout(r, 2000)); expect(document.head).toMatchSnapshot(); }); @@ -56,7 +57,6 @@ describe('usePatchFixedStyle hook', () => { paddingSize: 300, }) ); - await new Promise((r) => setTimeout(r, 2000)); expect(document.head).toMatchSnapshot(); }); @@ -70,7 +70,6 @@ describe('usePatchFixedStyle hook', () => { isHidden: true, }) ); - await new Promise((r) => setTimeout(r, 2000)); expect(document.head).toMatchSnapshot(); }); @@ -83,7 +82,6 @@ describe('usePatchFixedStyle hook', () => { paddingSize: 300, }) ); - await new Promise((r) => setTimeout(r, 2000)); expect(document.head).toMatchSnapshot(); }); From 36656695360d4b61598c67fbc42e05fe0e03021e Mon Sep 17 00:00:00 2001 From: tygao Date: Fri, 7 Jun 2024 15:38:12 +0800 Subject: [PATCH 8/9] add test case for hook unmount Signed-off-by: tygao --- .../use_patch_fixed_style.test.ts.snap | 56 +++++++++++++++++++ public/hooks/use_patch_fixed_style.test.ts | 21 +++++++ 2 files changed, 77 insertions(+) diff --git a/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap b/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap index fab4db5c..63cbfe94 100644 --- a/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap +++ b/public/hooks/__snapshots__/use_patch_fixed_style.test.ts.snap @@ -1,5 +1,61 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[`usePatchFixedStyle hook should not subscribe update after unmount 1`] = ` + + + + + + +`; + +exports[`usePatchFixedStyle hook should not subscribe update after unmount 2`] = ` + + + + + +`; + exports[`usePatchFixedStyle hook should patch corresponding left style when sidecarConfig$ pipe 1`] = `