Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add patch style for oui fixed components #203

Merged
merged 10 commits into from
Jun 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions public/chat_header_button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,6 +53,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => {
const core = useCore();
const flyoutFullScreen = sidecarDockedMode === SIDECAR_DOCKED_MODE.TAKEOVER;
const flyoutMountPoint = useRef(null);
usePatchFixedStyle();
Copy link
Member

@SuZhou-Joe SuZhou-Joe Jun 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking, the patched style should apply only when the chatbot is visible right? In current implementation, it will always patch the style to global even if the Chatbot is not mounted, which I think is not clean enough.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Patchstyle contains the logic that when chatbot is closed or docked takeover. When in these cases, we will update a empty style to remove patch style.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, so the sidecarConfig$ will emit a null value once the Chatbot is not visible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sidecar config will emit a new value with isHidden property is true.Then we will update inserted style with ""


useEffectOnce(() => {
const subscription = props.application.currentAppId$.subscribe((id) => setAppId(id));
Expand Down
89 changes: 89 additions & 0 deletions public/hooks/use_patch_fixed_style.ts
Original file line number Diff line number Diff line change
@@ -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<Text>();

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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we remove appended style after unmount?

};
}, []);
};

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;
});
Comment on lines +87 to +89
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
requestAnimationFrame(() => {
text.textContent = css;
});
requestAnimationFrame(() => {
if (config?.isHidden) { text.textContent = ''; return ; }
text.textContent = css;
});

Nit: I'd suggest explicitly making the textContent as empty.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here takeover dockedMode also takes the same effect as isHidden, should we also explicitly append?

}
Loading