Fixed few bugs present in NewHomePage look #443
Conversation
Signed-off-by: saimedhi <[email protected]>
saimedhi
requested review from
dbwiddis,
owaiskazi19,
joshpalis,
ohltyler,
amitgalitz,
jackiehanyang and
minalsha
as code owners
Signed-off-by: Sai Medhini Reddy Maryada <[email protected]>
|
@ohltyler, please take a look. In the next PR will reduce padding between panels in workflow detail page. |
There was a problem hiding this comment.
This has introduced a regression, the bottom panel seems to be getting cut off now. And because of this, if collapsing the Inspector panel, the button to reopen it is no longer visible.
| const handleLinkClick = (event: MouseEvent) => { | ||
| const anchorTag = (event.target as HTMLElement).closest( | ||
| 'a' | ||
| ) as HTMLAnchorElement | null; | ||
|
|
||
| const unlistenParentHistory = params.history.listen(() => { | ||
| if ( | ||
| hideInAppSideNavBar && | ||
| window.location.pathname.endsWith(`/app/${PLUGIN_ID}`) | ||
| ) { | ||
| window.location.href = EXPECTED_BASE_PATH; | ||
| if (anchorTag && anchorTag.href.endsWith(`/app/${PLUGIN_ID}`)) { | ||
| window.location.href = anchorTag.href + '#'; | ||
| window.dispatchEvent(new HashChangeEvent('hashchange')); | ||
| } | ||
| }); | ||
| }; | ||
| document.addEventListener('click', handleLinkClick); |
There was a problem hiding this comment.
If I understand correctly, this is performing this logic for every mouse click? This could add a performance hit, and feels like a workaround to me.
Can you take a look at AD plugins, or reach out to core OSD team to get better insight on how to fix the problem?
There was a problem hiding this comment.
This workaround is done to handle below scenario:
- In the NewHomePage - workflow details page, clicking search studio from sidenav will not redirect to flow framework dashboards home page (workflows list page).
- Reason : expected url by flow framework plugin is '/app/search-studio#' but getting '/app/search-studio'
There was a problem hiding this comment.
Understandable. But I still have the same concerns as mentioned. Can you follow up with AD plugin and/or core OSD team?
| const isWorkflowDetailPage = location.pathname.includes( | ||
| APP_PATH.WORKFLOW_DETAIL.split(':')[0] | ||
| ); | ||
| const sidebar = ( | ||
| <EuiPageSideBar | ||
| style={{ minWidth: 190 }} | ||
| hidden={hideInAppSideNavBar} | ||
| hidden={isWorkflowDetailPage || hideInAppSideNavBar} |
There was a problem hiding this comment.
Have you discussed with @opensearch-project/opensearch-ux on this change?
There was a problem hiding this comment.
Yes, UX team suggested to
- Hide navigation on the old look and feel for the Visual Editor.
- List view should show the navigation.
There was a problem hiding this comment.
@saimedhi Which navigation is this referring to? Does this only apply to the old look and feel?
There was a problem hiding this comment.
- The side nav shown below is only be visible on List view (old look).
| @@ -11,3 +11,8 @@ | |||
| height: 100%; | |||
| width: 100%; | |||
| } | |||
|
|
|||
| .panel-with-radius { | |||
There was a problem hiding this comment.
@saimedhi
Can we reuse the same styles from the new Discover?
It should be border-radius: 8px
This radius style should only be applied to the containers within the Visual Editor. The "Manage workflow/Create workflow" page should use the default containers.
|
Closing this PR as part of this is covered in #447. Will raise separate PRs to fix each bug. |
Description
screen-capture.23.webm
Issues Resolved
closes #442
Check List
--signoffBy 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.