-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Dashboard] Redesign Add Panel Experience #144418
Comments
Pinging @elastic/kibana-presentation (Team:Presentation) |
This is looking awesome! I'm wondering if we should remove the toolbar entirely to give that space back to the dashboard contents, and use a top nav item instead for the A couple other benefits of this approach off the top of my head are:
@andreadelrio, I know in your original designs you had a floating button on the bottom right of the dashboard. Is that still a design you're interested in using? |
++ for getting rid of the toolbar completely and finding another place for the button - either in the top nav bar or as a floating action (although floating actions tend to have issues with a11y, which is something we should be mindful of). It makes dashboards look so much cleaner (•̀ᴗ•́)و |
You know whats funny is this is moving back towards the design from Kibana 7.x. For example, here is some screen shots from 7.9. I always liked this menu better then the current toolbar. We should chat with design and understand why toolbar was added in the first place before removing it. I think we need to understand what problems it was trying to solve. Where clicking "Create new" opens |
cc @ryankeairns Maybe you can provide some more background on dashboard toolbar and the problems it was trying to solve? |
Thanks for finding these @nreese! You're right that we're moving in a direction that is more in line with pre 7.11 Dashboard. I believe that we can get the best of both worlds with this new design, especially if we take into consideration all of the reasons we moved to a toolbar in the first place! @ryankeairns would definitely know more than me, but from my recollection, the reasons that we created the toolbar are: Why did we create the toolbar
How this new design could address these items
There are absolutely reasons for the toolbar that I've forgotten or misrepresented! We should keep the discussion open to make sure that this design really does fit the best of both worlds. |
I like the idea of getting rid of the toolbar altogether. Using the top nav to house the |
@andreadelrio the top nav actually scrolls with the page! |
++ thanks Nathan for bringing this up.
One of the motivations for this proposal is actually to bring a little bit more of Lens into Dashboard to drive its adoption. We do this by directly presenting Lens' viz types to users. Also, we found that the word Lens itself (as included in the
++ I would also mention other "content" types. As we add more content types to Dashboard (think Controls) and in the future (panels, tabs and/or navigation embeddable) the toolbar starts suffering from growing pains and doesn't seem to scale that well (we found this in usability testing sessions). One of the main advantages of using the flyout to present all editor + content types available is that we can organize them in sections guiding users so they can more easily find them and figure out which one they need. I will bug Ryan to get his two cents here as he seems to have a lot of the background. |
One concern is "+ Add panel" button does not stand out at all and gets lost in the menu. I think "+ Add panel" button needs to be as strong or stronger call to action then the save button. This was a big driver of the toolbar and is reflected in #82906 |
That's a really good point. Cause when you're editing a dashboard, adding content to it is the thing you'd want to do, but saving is up there in importance as well. I wonder if we should introduce some more configuration options to the top nav menu, because it currently has only two levels of Maybe we need a design where whichever button is second in importance uses the |
To keep things consistent with other applications we could keep the |
This is fascinating to revisit. It seems most of the reasons/motivations are surfacing:
Regardless of which path you all head down, I would highly recommend testing out the solution(s). There are important nuances that may or may not promote the behavior we desire such as driving people to Lens. Other considerations:
I do like/see the potential of surfacing the viz type selector from Lens as the starting point in the flyout. That seems smart, dynamic in nature, and reduces some clicks. |
Related to the funkiness of the new panels being appended to the bottom of the dashboard: #97064 & #108951 Both of these could potentially be fixed with a scroll to view call somewhere in the |
One of the major pain points of the legacy visualizations is that users had to choose a viz type first. While building the viz you might discover the viz type isn't suitable for the data and you have to start over with a new viz type from scratch. Lens alleviates this by making it easier to switch viz types in the editor. And with Lens you shouldn't need to choose a viz type first. From the Lens documentation:
In this design the user on a dashboard chooses a viz type first that would open the Lens editor with the chosen viz type as the default. However, currently Lens will automatically change the viz type depending on which field(s) are added. So a user might have chosen a line chart, but Lens might change to a metric viz once the user adds a field. If the user enters Lens by choosing a viz type from the flyout, I think we have to tell the Lens editor "Hey, this user expected to create a line chart. Pin the viz type so that it doesn't change when the user adds a field. But also, allow the user to select a different viz type if they want to." I'm undecided on whether the flyout should list the Lens viz types. On one hand, I like giving users the option to choose the viz type first. But having Lens suggest the viz type based on the field is also helpful. I think we're removing that viz type suggestion for any user entering Lens from the flyout. |
Interesting. I had no idea that was the behavior of Lens. I know it shows a "Suggestions" section below the visualization but I didn't know it actually changes the type of visualization for users at times. I'd be interested to see this behavior in action to understand it better and see how we can accommodate this proposal to it. I tried replicating it myself in Lens but I was unsuccessful. |
@elastic/kibana-visualizations Is the above statement correct or am I misunderstanding? |
The Lens editor is trying to suggest the best chart depending on your fields. This might not be so obvious on the dataview mode because if I am not mistaken if you drop a field on the the default chart (stacked vertical bar), we are not suggesting something different (@dej611 correct me if I am wrong). Another example from the dataview mode. I start with waffle but as I am dropping fields it changes on a treemap With that being said I really like the idea to promote Lens and not the old editors, I am not sure though if it is a good idea to list all the charts that Lens supports. I don't have a super strong objection of course. I am open for discussion! |
I prefer having the Lens vis types listed out to highlight the breadth of charts available. With all the types listed out, there are many more options/routes that lead to Lens and a higher probability that the user will end up in Lens when building a new visualization. My concern is just having a single "Lens" option would potentially lead to the same issue as we had in the 7.9 vis modal where Lens is somewhat hidden amongst many other options and loses prominence. Even if we're somewhat reverting to an older design that we moved away from, I think the issue then wasn't that we had chart types individually listed, but that most of the options listed were legacy vis types with only one Lens option. I also can't think of a good way to refer to Lens as a vis option without actually saying "Lens" because it may not be apparent that's the option you choose to build a new vis and may lead to confusion when you also have options like "Custom visualization" or "Aggregation based visualization", even if they're further down the list. RE: Lens auto switching chart types from my observation, Lens will only switch to a different type when you add a field that is not compatible with the chart type you were on, and if the user wants to build a vis for that incompatible field, they probably wouldn't be upset that we didn't preserve their initial chart type selection. As long as we start with the selected chart type, I don't think it'll be too jarring when the chart type switches. Side note: Not sure how many people run into the chart switching/how big of a pain point this is, but maybe Lens could give some context when switching chart types by displaying info toasts explaining why the chart changed? |
From the engineering side, we'll need to ensure that we tackle #97010 alongside this redesign. This will be especially important for serverless, because we will want to configure which menu items show up here, and we want to put that complexity in the plugins that register the embeddable type rather than in Dashboard. |
When we build this, we should be extra careful about focus management, as we have the opportunity to speed up dashboard usage by focusing the relevant area. #64864 |
After conducting a dedicated survey to get user feedback on this idea, here are the designs we will be implementing. More details can be found in the Figma file. |
For the first implementation of this, @teresaalvarezsoler can you confirm if you agree with these changes? and what you think about the following questions? For the flyout
For the toolbar
|
Thank you @andreadelrio
I agree they seem too few options to have a flyout for serverless. I'm also worried that we will need to expose all the "Agg base" options for stateless in the flyout causing confusion to users. These options are currently hidden in the popover. Let's then keep the popover in the |
@teresaalvarezsoler that works for me! |
@teresaalvarezsoler To clarify, we're holding off on the flyout design until we can remove TSVB and Agg based vis, correct? For now, I will remove the quick actions and relabel the |
In serverless we have removed the agg based/TSVB editors and they don't appear on the dahsboard add panel popover. |
that's right @cqliu1 . Thank you! A couple of clarifications:
|
FWIW, if you're looking for user testing / feedback I would volunteer for this if you're needing someone. I've actually been a bit confused with the current (8.12) difference between say "Create visualization" and "Add panel > Lens". |
Noting here @nreese's suggestion that as part of the PR that fixes this issue, we should also write a functional test that counts the number of entries in this menu and fails if one is added. This way, we can be pinged for review on any embeddable addition to the Dashboard. |
## Summary Closes #144418 This PR introduces changes to the dashboard add panel selection functionality, so that panel selection would now happen from within a flyout, and as such panels are now grouped together logically. With this implementation any panel that is intended to show up within this new flyout is required to have either been registered leveraging the ui action trigger `ADD_PANEL_TRIGGER` and have it's `grouping` value defined or belong to a subset of visualization types (`PROMOTED`, `TOOLS`, and `LEGACY`) that would automatically get grouped. It's worth pointing out that because we can't control the order at which UI actions gets registered, we won't always get the the panel groups in the same order, for this specific reason ~a new optional property (`placementPriority`) has been added in~ the property `order` is now leveraged such that it allows a user registering a UI action define a relative weight for where they'd like their group to show up. All registered actions would be rendered in descending order considering all `order` defined, in the case where no order is defined `0` is assumed for the group. In addition an action which is registered without a group, would automatically get assigned into a default group titled "Other". The search implemented within the add panel is rudimentary, checking if the group titles and group item titles contain the input character; when a group title is matched the entire group is remains highlighted, in the case that the group isn't matched and it's just the group item, only said item is highlighted within it's group. ## Visuals #### Default view <img width="2560" alt="Screenshot 2024-06-10 at 17 44 17" src="https://github.com/elastic/kibana/assets/7893459/90aadf82-684a-4263-aecd-2843c3eff3c1"> #### Search match view <img width="2560" alt="Screenshot 2024-06-10 at 17 45 11" src="https://github.com/elastic/kibana/assets/7893459/5a766f29-a3b7-40e3-b1f7-8b423073cd87"> ##### P.S. This changes also includes changes to the display of certain panels; - ML group has a new title i.e. *Machine Learning and Analytics* - In serverless, the observability panels (SLO*) only shows as a selection choice in the observability project type. ### Checklist Delete any items that are not applicable to this PR. - [x] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) <!-- - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) --> - [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) <!-- ### 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](https://github.com/elastic/kibana/blob/main/RISK_MATRIX.mdx) | ### 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: Catherine Liu <[email protected]> Co-authored-by: kibanamachine <[email protected]>
Problem
As we add more panel types to dashboards, the current popover gets small and users need to scroll to find the options at the bottom making it harder to find the option that users are looking for.
Solution
We need a experience that scales as the number of panels grow and provides a better organization for the options so that users can find their panels more easily.
Requirements
The Aggregation based item should trigger the following modal:
Note these suggested changes in the modal compared to the existing modal:
(a) a more specific title (should we say
Legacy
in the title somehow?)(b) remove the “Select a different visualization link”. Closing the modal should be enough. The flyout should remain visible after closing the modal
(c) remove that image from the background, afaik that is an outdated UI detail that we don’t use often anymore.
Figma file
The text was updated successfully, but these errors were encountered: