Add patch style for oui fixed components #203
Conversation
Signed-off-by: tygao <[email protected]>
raintygao
requested review from
lezzago,
joshuali925,
anirudha,
ruanyl,
SuZhou-Joe,
xluo-aws,
gaobinlong,
Hailong-am and
wanglam
as code owners
| updateHeadStyle(config, textRef.current); | ||
| }); | ||
| return () => { | ||
| subscription.unsubscribe(); |
There was a problem hiding this comment.
Shall we remove appended style after unmount?
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
|
@raintygao Can you also add a screenshot in the description to better show what is the problem? |
Signed-off-by: tygao <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #203 +/- ##
==========================================
+ Coverage 90.02% 90.22% +0.20%
==========================================
Files 54 58 +4
Lines 1433 1545 +112
Branches 347 373 +26
==========================================
+ Hits 1290 1394 +104
- Misses 141 149 +8
Partials 2 2 ☔ View full report in Codecov by Sentry. |
Signed-off-by: tygao <[email protected]>
Sure. Updated. |
| @@ -52,6 +53,7 @@ export const HeaderChatButton = (props: HeaderChatButtonProps) => { | |||
| const core = useCore(); | |||
| const flyoutFullScreen = sidecarDockedMode === SIDECAR_DOCKED_MODE.TAKEOVER; | |||
| const flyoutMountPoint = useRef(null); | |||
| usePatchFixedStyle(); | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I see, so the sidecarConfig$ will emit a null value once the Chatbot is not visible?
There was a problem hiding this comment.
Sidecar config will emit a new value with isHidden property is true.Then we will update inserted style with ""
|
A PR in OSD that is related.cc @ruanyl @SuZhou-Joe opensearch-project/OpenSearch-Dashboards#6964 |
| requestAnimationFrame(() => { | ||
| text.textContent = css; | ||
| }); |
There was a problem hiding this comment.
| requestAnimationFrame(() => { | |
| text.textContent = css; | |
| }); | |
| requestAnimationFrame(() => { | |
| if (config?.isHidden) { text.textContent = ''; return ; } | |
| text.textContent = css; | |
| }); |
Nit: I'd suggest explicitly making the textContent as empty.
There was a problem hiding this comment.
I think here takeover dockedMode also takes the same effect as isHidden, should we also explicitly append?
Signed-off-by: tygao <[email protected]>
Signed-off-by: tygao <[email protected]>
* add patch style for oui fixed components Signed-off-by: tygao <[email protected]> * add remove child Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update test mock Signed-off-by: tygao <[email protected]> * add test for hook Signed-off-by: tygao <[email protected]> * remove useRef Signed-off-by: tygao <[email protected]> * use mock requestAnimationFrame to replace delay Signed-off-by: tygao <[email protected]> * add test case for hook unmount Signed-off-by: tygao <[email protected]> * update code structure Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> (cherry picked from commit 702f77e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md
* add patch style for oui fixed components Signed-off-by: tygao <[email protected]> * add remove child Signed-off-by: tygao <[email protected]> * doc: update changelog Signed-off-by: tygao <[email protected]> * update test mock Signed-off-by: tygao <[email protected]> * add test for hook Signed-off-by: tygao <[email protected]> * remove useRef Signed-off-by: tygao <[email protected]> * use mock requestAnimationFrame to replace delay Signed-off-by: tygao <[email protected]> * add test case for hook unmount Signed-off-by: tygao <[email protected]> * update code structure Signed-off-by: tygao <[email protected]> --------- Signed-off-by: tygao <[email protected]> (cherry picked from commit 702f77e) Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> # Conflicts: # CHANGELOG.md Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Description
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. Meanwhile, there is currently no way to provide config for these components at runtime.
This PR adds a hook to patch a style for all these already known components to make them compatible with sidecar.
Will consider to support config provider in OUI and refer this PR later.
ScreenShot
Before: the left or right property of flyout is always to be 0. The content width of bottomBar is always be 100%.
This still needs a PR in OSD to update sidecar z-index. opensearch-project/OpenSearch-Dashboards#6964
After: the left or right property of flyout will be updated accordingly. The bottomBar will be added paddingRight or paddingLeft style accordingly.
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.