[Dashboard] Focus on a single panel by cqliu1 · Pull Request #165417 · elastic/kibana

cqliu1 · 2023-08-31T22:37:30Z

Summary

This adds support for focusing a single panel on a dashboard while blurring/disabling all other panels.

Aug-31-2023.15-25-11.mp4

All interactions on blurred panels are disabled as well as any layout changes on either the focused or blurred panels, but the user still has access to controls, the editor toolbar, and unified search.

Note: For testing purposes, I have the focus behavior implemented with the Panel settings action, but I won't merge this PR with focus enabled for Panel settings. This behavior is meant for flyouts with inline editing capabilities, like the future ESQL inline editor flyout where changes apply in real-time without hitting Apply to see the changes.

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
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)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

cqliu1 · 2023-08-31T22:43:30Z

Do we want to disable controls, editor toolbar, and/or unified search access when focused on a single panel as well?
@teresaalvarezsoler @andreadelrio @stratoula

teresaalvarezsoler · 2023-09-01T08:52:54Z

Do we want to disable controls, editor toolbar, and/or unified search access when focused on a single panel as well?

When editing the panel settings it doesn't make much sense to filter visualizations but they don't hurt either so I would leave them accessible for now. In the future, we can reopen the discussion when users are able to edit panel-level filters from there.

Accessing the filters, controls, etc. definetely makes more sense when users are editing a visualization, @stratoula is it fine for ESQL visualizations? I think this doesn't work in Discover now.

stratoula · 2023-09-01T08:57:21Z

@teresaalvarezsoler not sure I have understood the question correctly but I will give it a shot. We have disabled the kql bar and the filter builder in Discover as you can do all these things with ES|QL. On a dashboard, where we don't have the ES|QL editor we have left the kql bar/filter builder which work fine with dataview and ESQL charts. With that being said, it makes sense to me to not disable these elements when inline editing a visualization.

teresaalvarezsoler · 2023-09-01T09:35:17Z

Great, then let's leave them accessible @cqliu1

cqliu1 · 2023-09-08T20:28:16Z

RE: a11y, it looks like even though mouse events are disabled for the blurred panels, you can still tab over to the buttons in the blurred panels before reaching the flyout. Do we want to prevent keyboard navigation to the blurred panels? If so, is this a blocker, or can we improve a11y in a follow up PR?

@teresaalvarezsoler @andreadelrio

elasticmachine · 2023-09-08T20:39:46Z

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

cqliu1 · 2023-09-20T12:02:18Z

@cqliu1, @teresaalvarezsoler, @stratoula: Per the designs, our suggestion was to leave the unified search interface enabled/accessible, but to disable the editor toolbar when a single panel is focused. Regarding custom controls, I don't think we made a decision regarding whether to enable/disable, so I'll leave that decision up to ya'll (I can see arguments for on both sides).

I've disabled the editor toolbar while a panel is focused, but I'll leave controls accessible.

ThomThomson · 2023-09-20T14:23:35Z

I made some small changes to the debounce effect on width changes:

I changed it from 250 ms to 100ms,
I skip the debounce entirely when the Dashboard has an overlay open.

This results in a smoother open / close experience.

…rd/focus-panel

cqliu1 · 2023-09-20T14:41:08Z

Looking good! The interactions of this (opening/closing the push flyout, rearrangement of the dashboard layout) feel a bit slow and not smooth to me. I wonder if there's a way to optimize this?

@andreadelrio With Devon's changes mentioned above that remove the debounce from overlays, opening and closing the flyout looks much smoother.

cqliu1 · 2023-09-20T15:31:56Z

In 3f02e1a, I figured out a way to programmatically remove blurred panels and nested elements from the tab order to disable keyboard navigation.

Sep-20-2023.11-29-16.mp4

cqliu1 · 2023-09-20T15:34:25Z

@elastic/appex-sharedux I'm not sure why you weren't automatically listed as a reviewer, but I enabled the isDisabled prop in the toolbar button so I could disable the editor toolbar when the ESQL flyout is open.

ThomThomson

Changes LGTM! Tested this in chrome, and everything looks good (especially the tab ordering). Also looked through the changes, and left one nit.

Additionally, can you think of a good way to test this, preferably with unit tests? Maybe checking for the presence of a class or a data test subj when focus panel is defined?

ThomThomson · 2023-09-20T15:51:10Z

...s/dashboard/public/dashboard_app/top_nav/controls_toolbar_button/controls_toolbar_button.tsx

+  isDisabled?: boolean;
+}
+
+export const ControlsToolbarButton: FC<Props> = ({ controlGroup, isDisabled }) => {


nit: why the use of FC here? Using FC like this implicitly adds the children prop, and this toolbar button doesn't actually take children. It's also a little cleaner to just declare the Props type inline instead of naming it.

ThomThomson · 2023-09-20T15:55:51Z

src/plugins/dashboard/public/dashboard_container/component/grid/dashboard_grid_item.tsx

      // https://github.com/react-grid-layout/react-grid-layout/issues/1241#issuecomment-658306889
      children,
      className,
-      style,


What was style used for? Nothing?

sebelga

Shared ux changes LGTM 👍

cqliu1 · 2023-09-20T16:56:51Z

src/plugins/dashboard/public/dashboard_container/component/grid/dashboard_grid_item.tsx


    return (
      <div
-        style={{ ...style, zIndex: focusedPanelId === id ? 2 : 'auto' }}


@ThomThomson Before style was destructured from props to override z-index on focusedPanelId which wasn't being used, and it doesn't seem necessary to change the z-index, so I removed the z-index style and removed the style previously destructured on line 46.

If style is passed in as a prop to this component, it will still be spread onto this div with the rest of the props.

andreadelrio

LGTM, thanks for the updates. Nice job on this!

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

gchaps · 2023-09-21T20:38:40Z

Can the menu item start with a verb, for example "Apply settings", so that it follows the format of the other items in the menu?

into dashboard/focus-panel

…rd/focus-panel

cqliu1 · 2023-09-25T18:26:32Z

Can the menu item start with a verb, for example "Apply settings", so that it follows the format of the other items in the menu?

@gchaps I'm not sure Apply settings feels right as the label here. That label sounds like it belongs a confirm/submit button on a form to apply changes that the user has made. Can we go with something like Adjust settings or Change settings? Is it okay to omit the word "panel" here? Maybe even Customize panel would sound better.

We renamed from Edit panel settings to Panel settings to avoid confusion between clicking Edit settings, which opens the panel settings flyout for editing the title, description, tags, and custom time range, and clicking Edit visualization/Lens/etc which redirects you to the associated editor.

gchaps · 2023-09-25T19:10:42Z

After talking with @cqliu1, we decided on Change panel settings.

ThomThomson · 2023-09-25T19:18:23Z

we decided on Change panel settings.

@gchaps @cqliu1 this item also exists in view mode, and in view mode you can't really change the settings. If we must have a verb, maybe we should switch it depending on the view mode?

cqliu1 · 2023-09-25T19:29:07Z

Yeah in edit mode you can change title, description, tags, along with the time range, but in view mode, you can only edit time range.

Edit mode

View mode

Let's just leave it as Panel settings.

kibana-ci · 2023-09-25T21:59:34Z

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id	before	after	diff
`canvas`	1011.7KB	1011.8KB	+57.0B
`dashboard`	355.5KB	356.5KB	+992.0B
`discover`	557.9KB	558.0KB	+31.0B
`embeddable`	12.0KB	12.0KB	-45.0B
`lens`	1.3MB	1.3MB	-7.0B
total			+1.0KB

Page load bundle

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

id	before	after	diff
`dashboard`	33.9KB	34.3KB	+416.0B

History

💚 Build #162027 succeeded 64ec4b2
💔 Build #161976 failed cc44000
💚 Build #160864 succeeded eb6659a
💔 Build #160450 failed 3f02e1a
💚 Build #160425 succeeded b07c722

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

cqliu1 marked this pull request as ready for review September 8, 2023 20:39

cqliu1 requested review from a team as code owners September 8, 2023 20:39

cqliu1 requested review from a team as code owners September 8, 2023 20:41

cqliu1 added the release_note:enhancement label Sep 8, 2023

cqliu1 requested review from a team as code owners September 8, 2023 20:41

cqliu1 requested a review from a team September 8, 2023 20:41

cqliu1 requested review from a team as code owners September 8, 2023 20:41

Disable editor toolbar while panel focused

d08cfa4

Remove console log

0650d5c

cqliu1 added 2 commits September 20, 2023 10:28

Skip debounce when overlays exist

3bdd2a1

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

0f76447

…rd/focus-panel

cqliu1 added 2 commits September 20, 2023 10:47

Restore panel settings flyout back to overlay instead of push

b07c722

Remove blurred items from tab order

3f02e1a

ThomThomson approved these changes Sep 20, 2023

View reviewed changes

sebelga approved these changes Sep 20, 2023

View reviewed changes

cqliu1 commented Sep 20, 2023

View reviewed changes

andreadelrio approved these changes Sep 20, 2023

View reviewed changes

stratoula mentioned this pull request Sep 21, 2023

[Lens] Inline editing of lens panels on a dashboard or canvas #166169

Merged

7 tasks

cqliu1 and others added 3 commits September 21, 2023 10:56

Skip debounce only if panel is focused

6758a0d

Undo FC typing

d4ac121

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

eb6659a

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

cqliu1 added 3 commits September 25, 2023 11:15

Rename var and added dashboard grid unit tests

17423e7

Merge branch 'dashboard/focus-panel' of https://github.com/cqliu1/kibana

7a0d3ee

into dashboard/focus-panel

Merge branch 'main' of https://github.com/elastic/kibana into dashboa…

cc44000

…rd/focus-panel

cqliu1 added 2 commits September 25, 2023 13:38

Fix jest test

64ec4b2

Add grid item unit tests

9fffa72

Conversation

cqliu1 commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Risk Matrix

For maintainers

Uh oh!

cqliu1 commented Aug 31, 2023

Uh oh!

teresaalvarezsoler commented Sep 1, 2023

Uh oh!

stratoula commented Sep 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

teresaalvarezsoler commented Sep 1, 2023

Uh oh!

cqliu1 commented Sep 8, 2023

Uh oh!

elasticmachine commented Sep 8, 2023

Uh oh!

cqliu1 commented Sep 20, 2023

Uh oh!

ThomThomson commented Sep 20, 2023

Uh oh!

cqliu1 commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cqliu1 commented Sep 20, 2023

Uh oh!

cqliu1 commented Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ThomThomson left a comment

Choose a reason for hiding this comment

Uh oh!

ThomThomson Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

ThomThomson Sep 20, 2023

Choose a reason for hiding this comment

Uh oh!

sebelga left a comment

Choose a reason for hiding this comment

Uh oh!

cqliu1 Sep 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andreadelrio left a comment

Choose a reason for hiding this comment

Uh oh!

gchaps commented Sep 21, 2023

Uh oh!

cqliu1 commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gchaps commented Sep 25, 2023

Uh oh!

ThomThomson commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cqliu1 commented Sep 25, 2023

Edit mode

View mode

Uh oh!

kibana-ci commented Sep 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

💚 Build Succeeded

Metrics [docs]

Async chunks

Page load bundle

History

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

cqliu1 commented Aug 31, 2023 •

edited

Loading

stratoula commented Sep 1, 2023 •

edited

Loading

cqliu1 commented Sep 20, 2023 •

edited

Loading

cqliu1 commented Sep 20, 2023 •

edited

Loading

cqliu1 Sep 20, 2023 •

edited

Loading

cqliu1 commented Sep 25, 2023 •

edited

Loading

ThomThomson commented Sep 25, 2023 •

edited

Loading

kibana-ci commented Sep 25, 2023 •

edited

Loading