Skip to content

Commit

Permalink
[8.6] [Discover] Fix Fields flyout and Create Field flyouts on mobile (
Browse files Browse the repository at this point in the history
…#145650) (#145789)

# Backport

This will backport the following commits from `main` to `8.6`:
- [[Discover] Fix Fields flyout and Create Field flyouts on mobile
(#145650)](#145650)

<!--- Backport version: 8.9.7 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT
[{"author":{"name":"Constance","email":"constancecchen@users.noreply.github.com"},"sourceCommit":{"committedDate":"2022-11-19T06:44:28Z","message":"[Discover]
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["bug","release_note:skip","v8.6.0","v8.7.0"],"number":145650,"url":"#145650
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"#145650
Fix Fields flyout and Create Field flyouts on mobile (#145650)\n\n##
Summary\r\n\r\n@jughosta flagged several responsive issues
in\r\nhttps://github.com//pull/144845#pullrequestreview-1183919709.\r\nTwo
of these were existing issues in Kibana main and need to
be\r\nbackported to 8.6, hence the separate PR.\r\n\r\nI recommend
turning off whitespace changes when viewing the diff, as\r\nmuch of the
wrapped/unwrapped code remained the same except
for\r\nindentation.\r\n\r\n### Create Field flyout:\r\n\r\n####
Before:\r\n<img width=\"760\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png\">\r\n\r\n####
After:\r\n<img width=\"779\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png\">\r\n\r\n###
Fields flyout (mobile only):\r\n\r\n#### Before:\r\n<img width=\"754\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png\">\r\n\r\n####
After:\r\n<img width=\"761\"
alt=\"\"\r\nsrc=\"https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png\">\r\n\r\n###
Checklist\r\n\r\n- [x] This renders correctly on smaller devices using a
responsive\r\nlayout. (You can test this [in
your\r\nbrowser](https://www.browserstack.com/guide/responsive-testing-on-local-server))\r\n-
[x] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\nCo-authored-by:
Kibana Machine
<42973632+kibanamachine@users.noreply.github.com>\r\nCo-authored-by:
Julia Rechkunova
<julia.rechkunova@elastic.co>","sha":"10c6649f4b28184fc07d41f081fac7e56c447a50"}}]}]
BACKPORT-->

Co-authored-by: Constance <constancecchen@users.noreply.github.com>
  • Loading branch information
kibanamachine and Constance committed Nov 19, 2022
1 parent 47b9663 commit 9e75cc2
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
EuiFlexItem,
EuiText,
EuiTitle,
useIsWithinMaxBreakpoint,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import { FormattedMessage } from '@kbn/i18n-react';
Expand Down Expand Up @@ -72,6 +73,8 @@ const FieldEditorFlyoutContentComponent = ({
const isEditingExistingField = !!fieldToEdit;
const { dataView, subfields$ } = useFieldEditorContext();

const isMobile = useIsWithinMaxBreakpoint('s');

const {
panel: { isVisible: isPanelVisible },
} = useFieldPreviewContext();
Expand Down Expand Up @@ -198,7 +201,7 @@ const FieldEditorFlyoutContentComponent = ({
<>
<FlyoutPanels.Group
flyoutClassName={euiFlyoutClassname}
maxWidth={1180}
maxWidth={isMobile ? false : 1180}
data-test-subj="fieldEditor"
fixedPanelWidths
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -110,20 +110,27 @@ export const Panels: React.FC<Props> = ({

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]);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

@include euiBreakpoint('xs', 's') {
width: 100%;
padding: $euiSize $euiSize 0 $euiSize;
padding: $euiSize;
background-color: $euiPageBackgroundColor;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,22 +320,19 @@ export function DiscoverSidebarResponsive(props: DiscoverSidebarResponsiveProps)
</h2>
</EuiTitle>
</EuiFlyoutHeader>
{/* Using only the direct flyout body class because we maintain scroll in a lower sidebar component. Needs a fix on the EUI side */}
<div className="euiFlyoutBody">
<DiscoverSidebar
{...props}
documents={documentState.result}
fieldCounts={fieldCounts.current}
fieldFilter={fieldFilter}
setFieldFilter={setFieldFilter}
alwaysShowActionButtons={true}
setFieldEditorRef={setFieldEditorRef}
closeFlyout={closeFlyout}
editField={editField}
createNewDataView={createNewDataView}
showDataViewPicker={true}
/>
</div>
<DiscoverSidebar
{...props}
documents={documentState.result}
fieldCounts={fieldCounts.current}
fieldFilter={fieldFilter}
setFieldFilter={setFieldFilter}
alwaysShowActionButtons={true}
setFieldEditorRef={setFieldEditorRef}
closeFlyout={closeFlyout}
editField={editField}
createNewDataView={createNewDataView}
showDataViewPicker={true}
/>
</EuiFlyout>
</EuiPortal>
)}
Expand Down

0 comments on commit 9e75cc2

Please sign in to comment.