Skip to content
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

Update and re-organize Dashboard panel action menu #66051

Closed
Tracked by #60433
mdefazio opened this issue May 11, 2020 · 16 comments · Fixed by #76497
Closed
Tracked by #60433

Update and re-organize Dashboard panel action menu #66051

mdefazio opened this issue May 11, 2020 · 16 comments · Fixed by #76497
Labels
enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Team:Visualizations Visualization editors, elastic-charts and infrastructure

Comments

@mdefazio
Copy link
Contributor

Describe the feature:
The current panel action menu has little visual prioritization and potentially a long list of options that causes the user to read through all options to decided which they want.
Re-organizing and segmenting the menu can help scanning through the options and promote common actions.

Current screenshots:
panel-action-menu

Update:
image

Describe a specific use case for the feature:
As we add more functionality to these panels, we need to make sure the structure scales and new features don't get lost in a long list of menu items.


Some questions to help guide this:

  1. What are all the different scenarios for this menu? View/Edit/Licensing
  2. What are the common actions that we should promote? Is there any customer feedback we can review?
  3. Do the icons still represent the action with which they are associated? Does this scale?
  4. What are some current restrictions we need to consider?

cc @AlonaNadler

@AlonaNadler
Copy link

Thanks @mdefazio that looks really great.
The action panel list does get longer and we need some sort of grouping and organization method
few things to add/change:

  • next release we will have also view in Discover, please add it under drill down
  • Replace panel is not that useful we can place it lower or within more
  • Changing the panel title is very useful we should call it out somewhere

@rayafratkina rayafratkina added Feature:Dashboard Dashboard related features Team:AppArch Team:Visualizations Visualization editors, elastic-charts and infrastructure labels May 12, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-arch (Team:AppArch)

@mdefazio
Copy link
Contributor Author

After discussing this internally, it's questionable that we need the subheadings. Instead we could simply utilize existing patterns and drop them into submenus. This helps shorten the height of the action menu (which was quite long using the subheads).

I've added some variations to try and find a compromise on length of menu and discoverability.

image

@timroes
Copy link
Contributor

timroes commented May 29, 2020

The menu as it is implemented today is a simple map over all registered actions for an embeddable that will be compiled in a list of items. To achieve something like shown in the screenshots at least the following changed will be needed:

  • Submenus need to be supported (again)
  • Categories (and thus grouping) need to be supported
  • Highlighting of specific actions to the top of the menu need to be supported
  • Additional badges and actions are needed on menu items

This panel is currently build by the uiActions menu and thus those extensions would need to be put on the App Archs roadmap. @alexh97 could you please take this into your next planning.

I in general am a bit concerned about the fact that this menu is purely under the control of uiActions and not the dashboard itself. It feels like a lot of actions here (like clone and remove) are purely related to dashboards, and having them run via the uiActions system might just be "unnecessary"? How are we in general making sure that dashboard specific actions are only shown on dashboards and not when the embeddable is used somewhere else? Also I have the feeling with a design like above the "highlighted" actions on top might be different depending on the context that the embeddable is (maybe Canvas want to highlight other actions than dashboard). Does it really still make sense having that panel be purely created by the uiActions plugin?

@stacey-gammon @ppisljar @majagrubic @ThomThomson

@stacey-gammon
Copy link
Contributor

stacey-gammon commented May 29, 2020

It feels like a lot of actions here (like clone and remove) are purely related to dashboards, and having them run via the uiActions system might just be "unnecessary"?

Clone and remove are actions registered by the dashboard, so they aren't owned by the uiActions system, but the uiActions system collects and renders them together.

How are we in general making sure that dashboard specific actions are only shown on dashboards and not when the embeddable is used somewhere else?

If an action requires a specific embeddable "parent", it will specify this in its isCompatible method. This is the clone isCompatible fn:

  public async isCompatible({ embeddable }: ClonePanelActionContext) {
    return Boolean(
      embeddable.getInput()?.viewMode !== ViewMode.VIEW &&
        embeddable.getRoot() &&
        embeddable.getRoot().isContainer &&
        embeddable.getRoot().type === DASHBOARD_CONTAINER_TYPE
    );
  }

So this will not show up for Embeddables anywhere other than on a dashboard that is in Edit mode.

Also I have the feeling with a design like above the "highlighted" actions on top might be different depending on the context that the embeddable is (maybe Canvas want to highlight other actions than dashboard).

++

Does it really still make sense having that panel be purely created by the uiActions plugin?

I think so. We just need to support different configurations. One question is - when you have multiple plugin authors attaching actions to this trigger, that may not know about eachother, who should dictate which actions go where? The dashboard, the generic embeddable system, or the specific embeddable implementation?

One way we could do this is is to add new triggers. In addition to PANEL_CONTEXT_MENU trigger, you could have a PANEL_CONTEXT_HIGHLIGHTED_ACTION trigger. The actions attached to that new trigger would be rendered differently by the embeddable panel.

If you wanted to say the container dictates which actions are highlighted (this would mean ML couldn't add their own special "highlighted" action), then you could possibly add this as input data to an embeddable, like highlightedActionIds. However, dashboard then needs to know about the actions it wants to highlight. I kind of like the first approach better.

I think what we need to move forward with this is a breakdown of the sections and how they might change from environment to environment based on different factors. Is it highlighted, misc, drilldowns, share? We could create separate triggers for all of them.

@streamich
Copy link
Contributor

Currently the context menu is generated out of a flat list of actions that are attached to CONTEXT_MENU_TRIGGER trigger.

It seems the simplest approach to support the above designs would be to add three more triggers to have:

  • CONTEXT_MENU_TRIGGER
  • FEATURED_CONTEXT_MENU_TRIGGER
  • DRILLDOWN_MENU_TRIGGER
  • SHARING_MENU_TRIGGER

image

@streamich
Copy link
Contributor

streamich commented Jun 2, 2020

Actually, FEATURED_CONTEXT_MENU_TRIGGER is not really necessary, we could display top 3 items from CONTEXT_MENU_TRIGGER as three buttons on the top.

Basically we just need to add SHARING_MENU_TRIGGER, which we wanted to do anyways (see SHARE_CONTEXT_MENU_TRIGGER).

And add DRILLDOWN_MENU_TRIGGER. We had CONTEXT_MENU_DRILLDOWNS_TRIGGER while developing Drilldowns and then reverted it back to a flat list.

@mdefazio
Copy link
Contributor Author

Here's an update on this:

image

I'm showing 2 different options (left - option A, right- option B).

We had a few more discussions around this on the design team. The consensus was that the flexibility of the vertical layout + nested lists made more sense for consistency and different use cases (option A).

Option B provides more prominence to the primary actions, but is obviously less flexible and perhaps something to consider further on. But option A felt like a logical improvement for a next step.

We can try and define a rule (similar to the EuiTable row actions) for a set amount of items at the top (likely 3) to help prioritize the actions and then provide the overflow nested menu if there are more available.

@timroes
Copy link
Contributor

timroes commented Jun 22, 2020

From a technical perspective, I think it would be very use-full to have items that can have sub-menus here @streamich. We currently experimenting with some Dashboard sections, where we would need an action here: "Move to other section", that should ideally directly lead to a submenu with all available sections, instead of throwing the user out of their flow with popping up a modal.

@Dosant
Copy link
Contributor

Dosant commented Jun 26, 2020

We discussed that a bit from implementation perspective, noting down 1 possible approach:


  1. We add ability to add options when attaching action to a trigger

Before:

public readonly addTriggerAction = <T extends TriggerId>(
    triggerId: T,
    action: Action<TriggerContextMapping[T]>
  )

After:

public readonly addTriggerAction = <T extends TriggerId>(
    triggerId: T,
    action: Action<TriggerContextMapping[T]>,
    options?: TriggerOptionsMapping[T]
  )

CONTEXT_MENU_TRIGGER will have optional options:

interface ContextMenuTriggerOptions {
  groupId?: 'more' | 'drilldowns' | 'share'
}

// ...

uiActions.addTriggerAction(CONTEXT_MENU_TRIGGER, editAction);
uiActions.addTriggerAction(CONTEXT_MENU_TRIGGER, createDrilldownAction, {groupId: 'drilldowns'});
  1. Then when rendering context menu, groupId and other options will be used to put action in correct place

⬆️ in the example groupId in ContextMenuTriggerOptions is static. So assumption is that embeddable panel which owns that trigger has to be aware of all possible groups.
We can improve this with extension point in embeddable plugin for adding groups ids dynamically.

@Dosant Dosant added enhancement New value added to drive a business result Feature:Embedding Embedding content via iFrame labels Jun 26, 2020
@streamich
Copy link
Contributor

The latest menu designs from our sync last week:

image

@AlonaNadler
Copy link

@streamich in the next dashboard phases it will no longer be required to save and name a panel when you create it from a dashboard. It means that many panel will be without a title. The ability to create a panel title need to be available in the edit mode. I think it needs to be second on that list right after edit visualization

@ThomThomson
Copy link
Contributor

That is a good point @AlonaNadler

I am also wondering about the placement of the 'add to library' and 'unlink from library' buttons. The 'unlink' button will be especially important when we first start to promote the 'by value' paradigm, because every panel will be 'by reference'

My current thought is that we could keep both actions under the the 'more' header to save space, but also hijack the 'by reference' badge from #75822 as a call to action which opens a context menu on click that provides an unlink option.

@streamich
Copy link
Contributor

streamich commented Sep 17, 2020

@AlonaNadler This is a bit tangential to this issue, but we have now discussed it few times in our syncs, so wanted to share here, too. Maybe we should refactor how the panel properties are edited. For example, it does not make much sense that when you edit a title, a modal opens with just one input field to edit the title. And when you edit the time range of the visualization another modal opens where you just edit the time range.

Now we are also are thinking to add some drilldown settings to each panel, for example, to enable in-chart "Explore underlying data".

And, for example when, you "inspect" the panel yet another flyout opens. And when you manage drilldowns, yet another flyout opens. And when you edit visualization, you are redirected to a completely different page.

Would be nice to see all those settings combined in one form. You just click "Edit", and you are presented with a flyout from the bottom of the screen, where you can edit title, set custom time range, manage drilldown settings, manage drilldowns, see inspector adapters.


I can put "Edit title" right after "Edit visualization" no problem.

@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServicesUx)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New value added to drive a business result Feature:Dashboard Dashboard related features Feature:Embedding Embedding content via iFrame Team:Visualizations Visualization editors, elastic-charts and infrastructure
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants