[Controls] Use new `panelMinWidth` prop in popovers #165397

Heenawter · 2023-08-31T19:27:00Z

Summary

This PR wraps up #162651 by fully migrating to the EuiInputPopover for all controls - specifically, this is made possible by the new panelMinWidth prop, which makes it so that the popover can now extend past the size of the input while maintaining the expected positioning.

Before	After
The popover was centered underneath the control on the smallest size:	The popover is left-aligned with the start of the input and expands to the right:
The range slider popover could not extend past the control width, regardless of how small that was:	The range slider popover now also has a minimum width, which makes it more useable on the smallest size:

Checklist

Unit or functional tests were updated or added to match the most common scenarios
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

For maintainers

This was checked for breaking API changes and was labeled appropriately

…-ref HEAD~1..HEAD --fix'

Heenawter · 2023-09-18T19:10:34Z

src/plugins/controls/public/control_group/control_group.scss

-    &:not(.controlFrameWrapper--grow) {
-      .controlFrame__labelToolTip {
-        max-width: 20%;
-      }
-    }


Now that the popover can extend past the input for range sliders, I don't think this specific styling is necessary anymore.

Before After

The control title takes up less space on the small size:

The control title takes up the same 40% even in the small size:

src/plugins/controls/public/constants.ts

Heenawter · 2023-09-18T19:14:23Z

src/plugins/controls/public/options_list/components/options_list_popover.tsx

@@ -43,36 +41,33 @@ export const OptionsListPopover = ({
  const [showOnlySelected, setShowOnlySelected] = useState(false);

  return (
-    <>
+    <div


I just removed the wrapping fragment, since it's unnecessary - no other changes were made here.

Heenawter · 2023-09-19T13:45:51Z

src/plugins/controls/public/options_list/components/options_list_popover.test.tsx

@@ -58,17 +57,6 @@ describe('Options list popover', () => {
    showOnlySelectedButton.simulate('click');
  };

-  test('available options list width responds to container size', async () => {


The EUI panelMinWidth applies to the portal that is created for the popover, and not to the popover itself; this portal does not exist from within the popover component, so this test is no longer relevant.

elasticmachine · 2023-09-19T13:46:35Z

Pinging @elastic/kibana-presentation (Team:Presentation)

Heenawter · 2023-09-19T14:48:22Z

@elasticmachine merge upstream

nickpeihl

lgtm!

code review and tested options list and range slider

nickpeihl · 2023-09-20T21:59:47Z

src/plugins/controls/public/options_list/components/options_list_control.tsx

        hasArrow={false}
        repositionOnScroll
        isOpen={isPopoverOpen}
        panelPaddingSize="none"
-        anchorPosition="downCenter"
+        panelMinWidth={MIN_POPOVER_WIDTH}
+        className="optionsList__inputButtonOverride"
        initialFocus={'[data-test-subj=optionsList-control-search-input]'}
        closePopover={() => optionsList.dispatch.setPopoverOpen(false)}
        aria-label={OptionsListStrings.popover.getAriaLabel(fieldName)}


Not blocking, but I can't figure out how to get this aria-label to read in VoiceOver.

Ahh, good point! Looks like the aria-label just doesn't do anything for EuiInputPopover - I tested this here: https://codesandbox.io/s/laughing-aryabhata-585w4s?file=/demo.js.

Fixed in e82ccfc7b88a9cb2f64a8a9c8e5b181e29db7f50 by passing it in as a panelProp instead 👍

src/plugins/controls/public/constants.ts

Heenawter · 2023-09-22T15:09:00Z

@elasticmachine merge upstream

kibana-ci · 2023-09-22T16:20:06Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 10ee17f

Failed CI Steps

Serverless Security Investigations Cypress Tests #1

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`controls`	131	132	+1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`controls`	188.7KB	188.3KB	-382.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`controls`	37.4KB	37.4KB	-2.0B

History

💚 Build #160026 succeeded c1157bb
💔 Build #159982 failed 2f8958a
💔 Build #159749 failed 1dbb8a3
💔 Build #159736 failed 1758d28
💔 Build #159732 failed 1e8b5692dbb51f29164f51c3fc0e3a3f1c854d03
💔 Build #159717 failed e036a06

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Heenawter

andreadelrio

Design changes LGTM, great improvements!

Closes elastic#164375 ## Summary This PR wraps up elastic#162651 by fully migrating to the `EuiInputPopover` for all controls - specifically, this is made possible by the new `panelMinWidth` prop, which makes it so that the popover can now extend past the size of the input **while maintaining** the expected positioning. | Before | After | |--------|--------| | The popover was centered underneath the control on the smallest size: ![image](https://github.com/elastic/kibana/assets/8698078/e2814ee2-6df6-47d6-925e-9f97cb8be2a5) | The popover is left-aligned with the start of the input and expands to the right: ![image](https://github.com/elastic/kibana/assets/8698078/7c698ef0-1534-43b6-ac95-9ae95f1c7613) | | The range slider popover could not extend past the control width, regardless of how small that was: ![image](https://github.com/elastic/kibana/assets/8698078/12e33967-b616-4f0a-9ded-4374d65a51b2) | The range slider popover now also has a minimum width, which makes it more useable on the smallest size: ![image](https://github.com/elastic/kibana/assets/8698078/2fb844db-8f5d-44d8-a6dc-c9cb95d5a4ea) | ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [x] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [x] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [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) ### For maintainers - [ ] This was checked for breaking API changes and was [labeled appropriately](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process) --------- Co-authored-by: kibanamachine <[email protected]>

Heenawter added 2 commits September 18, 2023 09:36

Switch options list over

3fb4397

Add minWidth to range slider + add constant

d14f6fb

Heenawter force-pushed the control-euiInputPopover-minWidth_2023-08-31 branch from 3ef74a9 to d14f6fb Compare September 18, 2023 15:36

Heenawter and others added 4 commits September 18, 2023 12:26

Fix linting

2c6049d

[CI] Auto-commit changed files from 'node scripts/precommit_hook.js -…

88220e3

…-ref HEAD~1..HEAD --fix'

Merge branch 'main' into control-euiInputPopover-minWidth_2023-08-31

d2de2c4

Remove specific small styling

e036a06

Heenawter commented Sep 18, 2023

View reviewed changes

src/plugins/controls/public/constants.ts Show resolved Hide resolved

Heenawter commented Sep 18, 2023

View reviewed changes

Fix failing test

1758d28

Heenawter force-pushed the control-euiInputPopover-minWidth_2023-08-31 branch from 1e8b569 to 1758d28 Compare September 18, 2023 19:57

Heenawter and others added 2 commits September 18, 2023 14:54

Remove unused test

1dbb8a3

Merge branch 'main' into control-euiInputPopover-minWidth_2023-08-31

2f8958a

Heenawter commented Sep 19, 2023

View reviewed changes

Heenawter marked this pull request as ready for review September 19, 2023 13:46

Heenawter requested review from a team as code owners September 19, 2023 13:46

Merge branch 'main' into control-euiInputPopover-minWidth_2023-08-31

c1157bb

Heenawter self-assigned this Sep 19, 2023

nickpeihl self-requested a review September 20, 2023 18:17

nickpeihl approved these changes Sep 20, 2023

View reviewed changes

Fix aria-label for options list

7675d64

Heenawter force-pushed the control-euiInputPopover-minWidth_2023-08-31 branch from e82ccfc to 7675d64 Compare September 22, 2023 15:08

Merge branch 'main' into control-euiInputPopover-minWidth_2023-08-31

10ee17f

andreadelrio approved these changes Sep 22, 2023

View reviewed changes

Heenawter merged commit bafb235 into elastic:main Sep 22, 2023

kibanamachine added v8.11.0 backport:skip This commit does not require backporting labels Sep 22, 2023

Heenawter deleted the control-euiInputPopover-minWidth_2023-08-31 branch September 22, 2023 19:03

Heenawter mentioned this pull request Oct 3, 2023

[Controls] Support date fields in options list #164362

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Controls] Use new `panelMinWidth` prop in popovers #165397

[Controls] Use new `panelMinWidth` prop in popovers #165397

Heenawter commented Aug 31, 2023 •

edited

Loading

Heenawter Sep 18, 2023 •

edited

Loading

Heenawter Sep 18, 2023

Heenawter Sep 19, 2023 •

edited

Loading

elasticmachine commented Sep 19, 2023

Heenawter commented Sep 19, 2023

nickpeihl left a comment

nickpeihl Sep 20, 2023

Heenawter Sep 22, 2023

Heenawter commented Sep 22, 2023

kibana-ci commented Sep 22, 2023

andreadelrio left a comment

[Controls] Use new panelMinWidth prop in popovers #165397

[Controls] Use new panelMinWidth prop in popovers #165397

Conversation

Heenawter commented Aug 31, 2023 • edited Loading

Summary

Checklist

For maintainers

Heenawter Sep 18, 2023 • edited Loading

Choose a reason for hiding this comment

Heenawter Sep 18, 2023

Choose a reason for hiding this comment

Heenawter Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

elasticmachine commented Sep 19, 2023

Heenawter commented Sep 19, 2023

nickpeihl left a comment

Choose a reason for hiding this comment

nickpeihl Sep 20, 2023

Choose a reason for hiding this comment

Heenawter Sep 22, 2023

Choose a reason for hiding this comment

Heenawter commented Sep 22, 2023

kibana-ci commented Sep 22, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

Page load bundle

History

andreadelrio left a comment

Choose a reason for hiding this comment

[Controls] Use new `panelMinWidth` prop in popovers #165397

[Controls] Use new `panelMinWidth` prop in popovers #165397

Heenawter commented Aug 31, 2023 •

edited

Loading

Heenawter Sep 18, 2023 •

edited

Loading

Heenawter Sep 19, 2023 •

edited

Loading