Expandable flyout width #170078

PhilippeOberti · 2023-10-27T22:13:09Z

Summary

This PR changes the way the kbn-expandable-flyout package handles multiple screen resolutions.

Prior implementation was pretty basic and was following EUI's basic flyout sizes, using a small size when only the right section was rendered, and a large size when both right and left section were rendered (using a 60%-40% for the content).

The new implementation is a lot smarter regarding handling screen resolution, while also answering customer's feedback that the right section's width is often too small to be usable.
Here are the biggest changes:

the right section's width will grow from a minimum of 380px to a maximum of 750px (unless the window size is smaller than 380px, in which case the flyout takes the size of the browser window)
the left section will take the remaining on the window width on resolutions lower than 1600px. For higher resolutions, the left section takes 80% of the window width, with a maximum set at 1500px

The PR also makes a couple of more minor changes:

adding responsive={false} in a lot of EuiFlexGroup within the flyout to avoid the UI to change on smaller screen (now that we have a minimum fixed width)
added unit tests to test the preview section

Right section only

Screen.Recording.2023-11-08.at.3.46.55.PM.mov

Right and left section

Screen.Recording.2023-11-08.at.3.47.35.PM.mov

https://github.com/elastic/security-team/issues/7896

PhilippeOberti · 2023-10-31T15:30:59Z

@elasticmachine merge upstream

elasticmachine · 2023-11-08T22:14:40Z

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

christineweng

Thank you for making these changes! Overall works great!

Observing a strange behavior with preview when width is between 1600px and 1900px, the calculated preview width is smaller than the right panel

Screen.Recording.2023-11-15.at.3.39.07.PM.mov

christineweng · 2023-11-15T19:31:49Z

packages/kbn-expandable-flyout/src/components/preview_section.test.tsx

+    expect(getByTestId(`${PREVIEW_SECTION_TEST_ID}BannerPanel`)).toHaveClass(
+      `euiPanel--${banner.backgroundColor}`
+    );
+    // expect(getByTestId(`${PREVIEW_SECTION_TEST_ID}BannerText`)).toHaveProperty(


is this test duplicate of the next one?

yeah forgot to remove that comment line, good catch thanks!

christineweng · 2023-11-15T19:39:25Z

packages/kbn-expandable-flyout/src/components/preview_section.tsx

-        bottom: 0;
-        right: 0;
-        left: ${left};
+        top: 4px;


the preview is now slightly smaller than the right panel, you can see the edges of right panel - is this an intended design choice?

I made this change on purpose, to get closer to the mocks

christineweng · 2023-11-15T20:57:47Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+    : 0;
+
+  // handle tiny screens by not going smaller than the window width
+  if (rightSectionWidth > windowWidth) rightSectionWidth = windowWidth;


as an alternative of adding an extra if here, you can take the smaller of calculated width and window width on line 69

Math.min(windowWidth, RIGHT_SECTION_MIN_WIDTH)

yup change made!

christineweng · 2023-11-15T20:58:29Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+    ? windowWidth < MIN_RESOLUTION_BREAKPOINT
+      ? RIGHT_SECTION_MIN_WIDTH
+      : RIGHT_SECTION_MIN_WIDTH +
+        (RIGHT_SECTION_MAX_WIDTH - RIGHT_SECTION_MIN_WIDTH) *


nit: maybe extract this ratio calculation as a variable?

@christineweng to solve this and the Math.min of your previous comment, this is the best I can come up with

let rightSectionWidth: number = 0; if (showRight) { if (windowWidth < MIN_RESOLUTION_BREAKPOINT) { // the right section's width will grow from 380px (at 992px resolution) while handling tiny screens by not going smaller than the window width rightSectionWidth = Math.min(RIGHT_SECTION_MIN_WIDTH, windowWidth); } else { const ratioWidth = (RIGHT_SECTION_MAX_WIDTH - RIGHT_SECTION_MIN_WIDTH) * ((windowWidth - MIN_RESOLUTION_BREAKPOINT) / (MAX_RESOLUTION_BREAKPOINT - MIN_RESOLUTION_BREAKPOINT)); // the right section's width will grow to 750px (at 1920px resolution) and will never go bigger than 750px in higher resolutions rightSectionWidth = Math.max(RIGHT_SECTION_MIN_WIDTH + ratioWidth, RIGHT_SECTION_MAX_WIDTH); } }

Do you prefer this over the previous code?

LGTM! good old if else saves the day :)

yeah, I actually pushed a commit changing right, left and preview section variables to be consistent :)

christineweng · 2023-11-15T21:13:09Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+    : 0;
+
+  // handle tiny screens by not going smaller than the window width
+  if (rightSectionWidth > windowWidth) rightSectionWidth = windowWidth;


I know small screen is not the main focus here, flagging it as observation: according to this line, if the windowWith is smaller than 380px, the right panel should take up the entire screen, during testing, I'm seeing a gap on the left which appears to be a SecurityPageWrapper

good observation and I noticed this as well during testing. The code says it should take the whole width but the UI doesn't reflect that. I hadn't gone as far of trying to understand why, because I considered this a minor annoyance for a width that we don't fully support...

I think it's something we can keep in mind for future work, maybe. The bug existed before this refactor so I'm ok to not worry about this for now, if you are as well or course!

christineweng · 2023-11-15T21:16:44Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+  if (rightSectionWidth > windowWidth) rightSectionWidth = windowWidth;
+
+  // never go bigger than 750px
+  if (rightSectionWidth > RIGHT_SECTION_MAX_WIDTH) rightSectionWidth = RIGHT_SECTION_MAX_WIDTH;


same as above, this check can be replaced by Math.min(calculated width, RIGHT_SECTION_MAX_WIDTH) on line 70

yup change made!

christineweng · 2023-11-15T21:27:01Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+    showRight && showLeft ? `${rightSectionWidth + leftSectionWidth}px` : `${rightSectionWidth}px`;
+
+  // preview section's width should only be similar to the right section
+  const previewSectionLeft: number =


curious why previewSectionLeft is different from leftSectionWidth?

good point, they're identical, I've updated the code accordingly

PhilippeOberti · 2023-11-15T23:12:04Z

packages/kbn-expandable-flyout/src/hooks/use_sections_sizes.ts

+
+  // preview section's width should only be similar to the right section.
+  // Though because the preview is rendered with an absolute position in the flyout, we calculate its left position instead of the width
+  let previewSectionLeft: number = 0;


@christineweng in theory, we could completely remove this variable and only rely on the leftSectionWdith. We could then also remove the showPreview prop.
I just feel like having this helps understanding. Are you ok with keeping it this way? To me it makes the code readable...

hmm... we shouldn't be relying on leftSectionWidth either because it could be zero, right? is it safe to assume if preview is open, then the right section is also open, the offset should be windowWidth - rightSectionWidth?

Yes I think we can assume that a preview section should always be displayed on top of a right section.

What I have now is

let previewSectionLeft: number = 0; if (showPreview) { // the preview section starts where the left section ends previewSectionLeft = leftSectionWidth; }

if showPreview is false then we have 0.
If it's true then we purely rely on the leftSectionWidth, which will be 0 of leftSection is false, and a non-0 value if leftSection is true.

christineweng

LGTM, thanks for making the changes!

Small ui nit: the right section of the header in insights are missing responsive={false}
here and here

…s width for different screen resolutions

kibana-ci · 2023-11-16T18:52:15Z

💚 Build Succeeded

Buildkite Build
Commit: e8572eb

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`securitySolution`	4694	4696	+2

Async chunks

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

id	before	after	diff
`securitySolution`	12.8MB	12.8MB	+3.3KB

History

💚 Build #176377 succeeded 1a54fe4
💛 Build #175968 was flaky e64a8d1
💛 Build #175866 was flaky f8f999b
💛 Build #175778 was flaky 73088a7
💛 Build #174463 was flaky 307e325
💔 Build #174376 failed edf7920

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

PhilippeOberti force-pushed the expandable-flyout-width branch from fc30711 to 2401755 Compare October 30, 2023 17:06

PhilippeOberti added release_note:enhancement Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team ci:cloud-deploy Create or update a Cloud deployment v8.12.0 labels Oct 30, 2023

PhilippeOberti force-pushed the expandable-flyout-width branch 4 times, most recently from 1274b74 to edf7920 Compare November 8, 2023 17:43

PhilippeOberti removed the ci:cloud-deploy Create or update a Cloud deployment label Nov 8, 2023

PhilippeOberti force-pushed the expandable-flyout-width branch from edf7920 to 307e325 Compare November 8, 2023 22:13

PhilippeOberti marked this pull request as ready for review November 8, 2023 22:14

PhilippeOberti requested a review from a team as a code owner November 8, 2023 22:14

PhilippeOberti requested a review from christineweng November 8, 2023 22:15

PhilippeOberti force-pushed the expandable-flyout-width branch 3 times, most recently from f8f999b to e64a8d1 Compare November 14, 2023 22:31

christineweng reviewed Nov 15, 2023

View reviewed changes

PhilippeOberti force-pushed the expandable-flyout-width branch from e64a8d1 to 1a54fe4 Compare November 15, 2023 23:10

PhilippeOberti commented Nov 15, 2023

View reviewed changes

PhilippeOberti requested a review from christineweng November 16, 2023 05:23

christineweng approved these changes Nov 16, 2023

View reviewed changes

PhilippeOberti added 3 commits November 16, 2023 11:36

[Security Solution] updating how kbn-expandable-flyout package handle…

f7fdcf0

…s width for different screen resolutions

PR comments

ed30633

finale PR comments

e8572eb

PhilippeOberti force-pushed the expandable-flyout-width branch from 1a54fe4 to e8572eb Compare November 16, 2023 17:44

PhilippeOberti merged commit c3ac89c into elastic:main Nov 16, 2023
26 checks passed

kibanamachine added the backport:skip This commit does not require backporting label Nov 16, 2023

PhilippeOberti deleted the expandable-flyout-width branch November 16, 2023 18:56

PhilippeOberti mentioned this pull request Mar 26, 2024

[Security Solution] [UX feature request] Make Event/Alert Details View resizable for Table view/JSON view #179254

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expandable flyout width #170078

Expandable flyout width #170078

PhilippeOberti commented Oct 27, 2023 •

edited

Loading

PhilippeOberti commented Oct 31, 2023

elasticmachine commented Nov 8, 2023

christineweng left a comment

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

PhilippeOberti Nov 15, 2023 •

edited

Loading

christineweng Nov 15, 2023

PhilippeOberti Nov 15, 2023

christineweng left a comment

kibana-ci commented Nov 16, 2023

Expandable flyout width #170078

Expandable flyout width #170078

Conversation

PhilippeOberti commented Oct 27, 2023 • edited Loading

Summary

PhilippeOberti commented Oct 31, 2023

elasticmachine commented Nov 8, 2023

christineweng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PhilippeOberti Nov 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

christineweng left a comment

Choose a reason for hiding this comment

kibana-ci commented Nov 16, 2023

💚 Build Succeeded

Metrics [docs]

Module Count

Async chunks

History

PhilippeOberti commented Oct 27, 2023 •

edited

Loading

PhilippeOberti Nov 15, 2023 •

edited

Loading