From 10c6649f4b28184fc07d41f081fac7e56c447a50 Mon Sep 17 00:00:00 2001 From: Constance Date: Fri, 18 Nov 2022 22:44:28 -0800 Subject: [PATCH] [Discover] Fix Fields flyout and Create Field flyouts on mobile (#145650) ## Summary @jughosta flagged several responsive issues in https://github.com/elastic/kibana/pull/144845#pullrequestreview-1183919709. Two of these were existing issues in Kibana main and need to be backported to 8.6, hence the separate PR. I recommend turning off whitespace changes when viewing the diff, as much of the wrapped/unwrapped code remained the same except for indentation. ### Create Field flyout: #### Before: #### After: ### Fields flyout (mobile only): #### Before: #### After: ### Checklist - [x] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [x] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Julia Rechkunova --- .../field_editor_flyout_content.tsx | 5 ++- .../flyout_panels/flyout_panels.tsx | 37 +++++++++++-------- .../components/sidebar/discover_sidebar.scss | 2 +- .../sidebar/discover_sidebar_responsive.tsx | 29 +++++++-------- 4 files changed, 40 insertions(+), 33 deletions(-) diff --git a/src/plugins/data_view_field_editor/public/components/field_editor_flyout_content.tsx b/src/plugins/data_view_field_editor/public/components/field_editor_flyout_content.tsx index edd0a7bc70b62..a775f537ecaf7 100644 --- a/src/plugins/data_view_field_editor/public/components/field_editor_flyout_content.tsx +++ b/src/plugins/data_view_field_editor/public/components/field_editor_flyout_content.tsx @@ -13,6 +13,7 @@ import { EuiFlexItem, EuiText, EuiTitle, + useIsWithinMaxBreakpoint, } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; import { FormattedMessage } from '@kbn/i18n-react'; @@ -72,6 +73,8 @@ const FieldEditorFlyoutContentComponent = ({ const isEditingExistingField = !!fieldToEdit; const { dataView, subfields$ } = useFieldEditorContext(); + const isMobile = useIsWithinMaxBreakpoint('s'); + const { panel: { isVisible: isPanelVisible }, } = useFieldPreviewContext(); @@ -198,7 +201,7 @@ const FieldEditorFlyoutContentComponent = ({ <> diff --git a/src/plugins/data_view_field_editor/public/components/flyout_panels/flyout_panels.tsx b/src/plugins/data_view_field_editor/public/components/flyout_panels/flyout_panels.tsx index 95fb44b293e00..80d697e78e03c 100644 --- a/src/plugins/data_view_field_editor/public/components/flyout_panels/flyout_panels.tsx +++ b/src/plugins/data_view_field_editor/public/components/flyout_panels/flyout_panels.tsx @@ -14,7 +14,7 @@ import React, { useMemo, useLayoutEffect, } from 'react'; -import { EuiFlexGroup, EuiFlexGroupProps } from '@elastic/eui'; +import { EuiFlexGroup, EuiFlexGroupProps, EuiFlyoutProps } from '@elastic/eui'; import './flyout_panels.scss'; @@ -47,7 +47,7 @@ export interface Props { * The total max width with all the panels in the DOM * Corresponds to the "maxWidth" prop passed to the EuiFlyout */ - maxWidth: number; + maxWidth: EuiFlyoutProps['maxWidth']; /** The className selector of the flyout */ flyoutClassName: string; /** The size between the panels. Corresponds to EuiFlexGroup gutterSize */ @@ -110,20 +110,27 @@ export const Panels: React.FC = ({ let currentWidth: number; - if (fixedPanelWidths) { - const totalWidth = Object.values(panels).reduce((acc, { width = 0 }) => acc + width, 0); - currentWidth = Math.min(maxWidth, totalWidth); - // As EUI declares both min-width and max-width on the .euiFlyout CSS class - // we need to override both values - flyoutDOMelement.style.minWidth = `${limitWidthToWindow(currentWidth, window)}px`; - flyoutDOMelement.style.maxWidth = `${limitWidthToWindow(currentWidth, window)}px`; + if (typeof maxWidth === 'number') { + if (fixedPanelWidths) { + const totalWidth = Object.values(panels).reduce((acc, { width = 0 }) => acc + width, 0); + currentWidth = Math.min(maxWidth, totalWidth); + // As EUI declares both min-width and max-width on the .euiFlyout CSS class + // we need to override both values + flyoutDOMelement.style.minWidth = `${limitWidthToWindow(currentWidth, window)}px`; + flyoutDOMelement.style.maxWidth = `${limitWidthToWindow(currentWidth, window)}px`; + } else { + const totalPercentWidth = Math.min( + 100, + Object.values(panels).reduce((acc, { width = 0 }) => acc + width, 0) + ); + currentWidth = (maxWidth * totalPercentWidth) / 100; + flyoutDOMelement.style.maxWidth = `${limitWidthToWindow(currentWidth, window)}px`; + } } else { - const totalPercentWidth = Math.min( - 100, - Object.values(panels).reduce((acc, { width = 0 }) => acc + width, 0) - ); - currentWidth = (maxWidth * totalPercentWidth) / 100; - flyoutDOMelement.style.maxWidth = `${limitWidthToWindow(currentWidth, window)}px`; + // maxWidth is false on smaller mobile screens when the preview panel is unused. + // Unset custom min/max widths and let EUI's default 90vw width be used + flyoutDOMelement.style.minWidth = ''; + flyoutDOMelement.style.maxWidth = ''; } }, [panels, maxWidth, fixedPanelWidths, flyoutClassName, flyoutDOMelement]); diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.scss b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.scss index 2a4f6c557e64e..4a0f048947706 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.scss +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar.scss @@ -8,7 +8,7 @@ @include euiBreakpoint('xs', 's') { width: 100%; - padding: $euiSize $euiSize 0 $euiSize; + padding: $euiSize; background-color: $euiPageBackgroundColor; } } diff --git a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx index 9e8ad96f9abb6..73271a0260709 100644 --- a/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx +++ b/src/plugins/discover/public/application/main/components/sidebar/discover_sidebar_responsive.tsx @@ -320,22 +320,19 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps) - {/* Using only the direct flyout body class because we maintain scroll in a lower sidebar component. Needs a fix on the EUI side */} -
- -
+ )}