Skip to content

Commit

Permalink
[8.x] [Unified Field List] Fix issue where Unified Field List field p…
Browse files Browse the repository at this point in the history
…opover gets cut off (elastic#195147) (elastic#195512)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Unified Field List] Fix issue where Unified Field List field popover
gets cut off (elastic#195147)](elastic#195147)

<!--- Backport version: 9.4.3 -->

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

<!--BACKPORT [{"author":{"name":"Davis
McPhee","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-08T21:43:52Z","message":"[Unified
Field List] Fix issue where Unified Field List field popover gets cut
off (elastic#195147)\n\n## Summary\r\n\r\nThis PR fixes an issue where the
Unified Field List field popover can\r\nget cut off if its contents
exceed the view height. Now, instead of\r\ncutting off the popover, we
limit the content height to `90vh` and make\r\nthe main section
scrollable.\r\n\r\nBefore (from elastic#194313 test
failure):\r\n\r\n![image](https://github.com/user-attachments/assets/5927a899-a18a-4431-bd1d-6bb2682cd004)\r\n\r\nAfter:\r\n\r\n![scroll](https://github.com/user-attachments/assets/5071a52b-fbf4-4d05-96de-61858d9e5598)\r\n\r\nFlaky
test
runs:\r\n-\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7098\r\n\r\nFixes
elastic#194313.\r\nFixes elastic#193934.\r\nFixes elastic#193781.\r\n\r\n###
Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\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-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"26d5634b23fc76d7a87aaba53892dabb31866d54","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:DataDiscovery","backport:prev-minor","backport:prev-major"],"title":"[Unified
Field List] Fix issue where Unified Field List field popover gets cut
off","number":195147,"url":"https://github.com/elastic/kibana/pull/195147","mergeCommit":{"message":"[Unified
Field List] Fix issue where Unified Field List field popover gets cut
off (elastic#195147)\n\n## Summary\r\n\r\nThis PR fixes an issue where the
Unified Field List field popover can\r\nget cut off if its contents
exceed the view height. Now, instead of\r\ncutting off the popover, we
limit the content height to `90vh` and make\r\nthe main section
scrollable.\r\n\r\nBefore (from elastic#194313 test
failure):\r\n\r\n![image](https://github.com/user-attachments/assets/5927a899-a18a-4431-bd1d-6bb2682cd004)\r\n\r\nAfter:\r\n\r\n![scroll](https://github.com/user-attachments/assets/5071a52b-fbf4-4d05-96de-61858d9e5598)\r\n\r\nFlaky
test
runs:\r\n-\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7098\r\n\r\nFixes
elastic#194313.\r\nFixes elastic#193934.\r\nFixes elastic#193781.\r\n\r\n###
Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\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-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"26d5634b23fc76d7a87aaba53892dabb31866d54"}},"sourceBranch":"main","suggestedTargetBranches":[],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195147","number":195147,"mergeCommit":{"message":"[Unified
Field List] Fix issue where Unified Field List field popover gets cut
off (elastic#195147)\n\n## Summary\r\n\r\nThis PR fixes an issue where the
Unified Field List field popover can\r\nget cut off if its contents
exceed the view height. Now, instead of\r\ncutting off the popover, we
limit the content height to `90vh` and make\r\nthe main section
scrollable.\r\n\r\nBefore (from elastic#194313 test
failure):\r\n\r\n![image](https://github.com/user-attachments/assets/5927a899-a18a-4431-bd1d-6bb2682cd004)\r\n\r\nAfter:\r\n\r\n![scroll](https://github.com/user-attachments/assets/5071a52b-fbf4-4d05-96de-61858d9e5598)\r\n\r\nFlaky
test
runs:\r\n-\r\nhttps://buildkite.com/elastic/kibana-flaky-test-suite-runner/builds/7098\r\n\r\nFixes
elastic#194313.\r\nFixes elastic#193934.\r\nFixes elastic#193781.\r\n\r\n###
Checklist\r\n\r\n- [ ] Any text added follows [EUI's
writing\r\nguidelines](https://elastic.github.io/eui/#/guidelines/writing),
uses\r\nsentence case text and includes
[i18n\r\nsupport](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)\r\n-
[
]\r\n[Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html)\r\nwas
added for features that require explanation or tutorials\r\n- [ ] [Unit
or
functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere
updated or added to match the most common scenarios\r\n- [x] [Flaky
Test\r\nRunner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1)
was\r\nused on any tests changed\r\n- [ ] Any UI touched in this PR is
usable by keyboard only (learn more\r\nabout [keyboard
accessibility](https://webaim.org/techniques/keyboard/))\r\n- [ ] Any UI
touched in this PR does not create any new axe failures\r\n(run axe in
browser:\r\n[FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/),\r\n[Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US))\r\n-
[ ] If a plugin configuration key changed, check if it needs to
be\r\nallowlisted in the cloud and added to the
[docker\r\nlist](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker)\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-
[ ] This was checked for
[cross-browser\r\ncompatibility](https://www.elastic.co/support/matrix#matrix_browsers)\r\n\r\n###
For maintainers\r\n\r\n- [ ] This was checked for breaking API changes
and was
[labeled\r\nappropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)\r\n\r\n---------\r\n\r\nCo-authored-by:
kibanamachine
<[email protected]>","sha":"26d5634b23fc76d7a87aaba53892dabb31866d54"}}]}]
BACKPORT-->

Co-authored-by: Davis McPhee <[email protected]>
  • Loading branch information
kibanamachine and davismcphee authored Oct 8, 2024
1 parent 30a25c8 commit 31e9a3b
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 117 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,33 @@
*/

import React from 'react';
import { EuiPopover, EuiPopoverProps, EuiPopoverTitle } from '@elastic/eui';
import {
EuiFlexGroup,
EuiFlexItem,
EuiPopover,
EuiPopoverProps,
EuiPopoverTitle,
} from '@elastic/eui';
import './field_popover.scss';
import { euiThemeVars } from '@kbn/ui-theme';

export interface FieldPopoverProps extends EuiPopoverProps {
renderHeader?: () => React.ReactNode;
renderContent?: () => React.ReactNode;
renderFooter?: () => React.ReactNode;
}

export const FieldPopover: React.FC<FieldPopoverProps> = ({
isOpen,
closePopover,
renderHeader,
renderContent,
renderFooter,
...otherPopoverProps
}) => {
let header = null;
let content = null;
let header: React.ReactNode | null = null;
let content: React.ReactNode | null = null;
let footer: React.ReactNode | null = null;

if (isOpen) {
try {
Expand All @@ -40,6 +50,13 @@ export const FieldPopover: React.FC<FieldPopoverProps> = ({
// eslint-disable-next-line no-console
console.error(error);
}

try {
footer = renderFooter?.() || null;
} catch (error) {
// eslint-disable-next-line no-console
console.error(error);
}
}

return (
Expand All @@ -54,10 +71,27 @@ export const FieldPopover: React.FC<FieldPopoverProps> = ({
{...otherPopoverProps}
>
{isOpen && (
<>
{content && header ? <EuiPopoverTitle>{header}</EuiPopoverTitle> : header}
{content}
</>
<EuiFlexGroup gutterSize="none" direction="column" css={{ maxHeight: '90vh' }}>
{Boolean(header) && (
<EuiFlexItem grow={false}>
{content ? <EuiPopoverTitle>{header}</EuiPopoverTitle> : header}
</EuiFlexItem>
)}
{content ? (
<EuiFlexItem
className="eui-yScrollWithShadows"
css={{
padding: euiThemeVars.euiSize,
margin: `-${euiThemeVars.euiSize}`,
}}
>
{content}
</EuiFlexItem>
) : (
content
)}
{Boolean(footer) && <EuiFlexItem grow={false}>{footer}</EuiFlexItem>}
</EuiFlexGroup>
)}
</EuiPopover>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,22 +309,39 @@ function UnifiedFieldListItemComponent({
/>
</>
)}

{searchMode === 'documents' && !!services.uiActions && (
<FieldPopoverFooter
field={field}
dataView={dataView}
multiFields={rawMultiFields}
trackUiMetric={trackUiMetric}
contextualFields={workspaceSelectedFieldNames}
originatingApp={stateService.creationOptions.originatingApp}
uiActions={services.uiActions}
/>
)}
</>
);
};

const renderFooter = useMemo(() => {
const uiActions = services.uiActions;

if (searchMode !== 'documents' || !uiActions) {
return;
}

return () => (
<FieldPopoverFooter
field={field}
dataView={dataView}
multiFields={rawMultiFields}
trackUiMetric={trackUiMetric}
contextualFields={workspaceSelectedFieldNames}
originatingApp={stateService.creationOptions.originatingApp}
uiActions={uiActions}
/>
);
}, [
dataView,
field,
rawMultiFields,
searchMode,
services.uiActions,
stateService.creationOptions.originatingApp,
trackUiMetric,
workspaceSelectedFieldNames,
]);

const value = useMemo(
() => ({
id: field.name,
Expand Down Expand Up @@ -393,6 +410,7 @@ function UnifiedFieldListItemComponent({
? renderPopover
: undefined
}
renderFooter={renderFooter}
/>
);
}
Expand Down
3 changes: 2 additions & 1 deletion packages/kbn-unified-field-list/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@
"@kbn/visualization-utils",
"@kbn/esql-utils",
"@kbn/search-types",
"@kbn/fields-metadata-plugin"
"@kbn/fields-metadata-plugin",
"@kbn/ui-theme"
],
"exclude": ["target/**/*"]
}
Loading

0 comments on commit 31e9a3b

Please sign in to comment.