Skip to content

Commit

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

## Summary

@jughosta flagged several responsive issues in
elastic#144845 (review).
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:
<img width="760" alt=""
src="https://user-images.githubusercontent.com/549407/202580476-610e3f54-eabb-4859-a49d-19d0608be787.png">

#### After:
<img width="779" alt=""
src="https://user-images.githubusercontent.com/549407/202580282-886435e7-00cc-4569-943d-fff471b3af7b.png">

### Fields flyout (mobile only):

#### Before:
<img width="754" alt=""
src="https://user-images.githubusercontent.com/549407/202580465-917ef121-d0eb-4188-bcf7-2b08edb5dd49.png">

#### After:
<img width="761" alt=""
src="https://user-images.githubusercontent.com/549407/202580275-820b9420-a8da-458e-bbb4-5f1cfb57ba99.png">

### 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 <[email protected]>
Co-authored-by: Julia Rechkunova <[email protected]>
  • Loading branch information
3 people authored Nov 19, 2022
1 parent bd49b7d commit 10c6649
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 10c6649

Please sign in to comment.