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

Support red dot icons on buttons with call backs #3046

Closed
lezzago opened this issue Dec 7, 2022 · 8 comments
Closed

Support red dot icons on buttons with call backs #3046

lezzago opened this issue Dec 7, 2022 · 8 comments
Assignees
Labels
enhancement New feature or request invalid This doesn't seem right wontfix This will not be worked on

Comments

@lezzago
Copy link
Member

lezzago commented Dec 7, 2022

Is your feature request related to a problem? Please describe.

On the Dashboard page with multiple visualizations, we want the 3 dot button to show a red icon if there action to take based on some call back the button contains. Based on #2880, we want to show the red icon when there are active alerts from the alerting plugin.

red dot on dashboards

Describe the solution you'd like

I would like the ability to register call backs or associate call backs from the external plugins (in this case Alerting plugin) to the buttons and icons, so they know to call the alerting plugin registered callback function to know if it should display a red dot or not.

@lezzago lezzago added the enhancement New feature or request label Dec 7, 2022
@lezzago
Copy link
Member Author

lezzago commented Dec 9, 2022

Proposed solution:

  • Update the EmbeddableOutput object to support a new boolean attribute called something like indicateActionToTake.
  • Make sure that if indicateActionToTake is true, update the 3 dot button to show the red dot

For testing purposes:

  • In the updateHandler function, make random scenarios to update indicateActionToTake to true to ensure the red dot it being displayed and removed correctly.

This work will later have a dependency on #2893

@ashwin-pc
Copy link
Member

@lezzago thanks for opening this issue. I like the approach you've proposed of adding the boolean flag to the embeddable to indicate a notification. How do you plan on setting this flag since the embeddable is essentially rendered by the visualization plugin. Also I see a second indicator in the context menu, does this issue not cover that scenario?

@lezzago
Copy link
Member Author

lezzago commented Dec 9, 2022

@ashwin-pc the idea is that as the updateHandler function, we will retrieve the alerts and anomalies from the saved objects and then use that information to decide if there should be a red indicator on the 3 dot button.

@ashwin-pc
Copy link
Member

Thanks for the context, that makes sense. Can we call it something simpler though? indicateActionToTake seems like a mouthful and not immediately clear about what it does. hasNotifications, hasEvents, or even something as simple as showNotificationIndcator that makes it explicit about what the flag does.

Since these are API changes I want to make sure that we get the names right since they will be annoying to walk back on.

@joshuarrrr
Copy link
Member

joshuarrrr commented Dec 9, 2022

@lezzago Have you already looked at these? I haven't traced their usage yet, but from the names it sure sounds like what we're trying to do here:

export const PANEL_BADGE_TRIGGER = 'PANEL_BADGE_TRIGGER';
export const panelBadgeTrigger: Trigger<'PANEL_BADGE_TRIGGER'> = {
id: PANEL_BADGE_TRIGGER,
title: 'Panel badges',
description: 'Actions appear in title bar when an embeddable loads in a panel.',
};
export const PANEL_NOTIFICATION_TRIGGER = 'PANEL_NOTIFICATION_TRIGGER';
export const panelNotificationTrigger: Trigger<'PANEL_NOTIFICATION_TRIGGER'> = {
id: PANEL_NOTIFICATION_TRIGGER,
title: 'Panel notifications',
description: 'Actions appear in top-right corner of a panel.',
};

and

private async refreshBadges() {
let badges = await this.props.getActions(PANEL_BADGE_TRIGGER, {
embeddable: this.props.embeddable,
});
if (!this.mounted) return;
const { disabledActions } = this.props.embeddable.getInput();
if (disabledActions) {
badges = badges.filter((badge) => disabledActions.indexOf(badge.id) === -1);
}
this.setState({
badges,
});
}
private async refreshNotifications() {
let notifications = await this.props.getActions(PANEL_NOTIFICATION_TRIGGER, {
embeddable: this.props.embeddable,
});
if (!this.mounted) return;
const { disabledActions } = this.props.embeddable.getInput();
if (disabledActions) {
notifications = notifications.filter((badge) => disabledActions.indexOf(badge.id) === -1);
}
this.setState({
notifications,
});
}

@joshuarrrr
Copy link
Member

We also already have rendering logic for EUINotificationBadges as part of the panel header. I'd advocate for figuring out how to use this existing pattern and interface rather than restyling the context menu button to have a dot indicator:

function renderNotifications(
notifications: Array<Action<EmbeddableContext>>,
embeddable: IEmbeddable
) {
return notifications.map((notification) => {
const context = { embeddable };
let badge = notification.MenuItem ? (
React.createElement(uiToReactComponent(notification.MenuItem))
) : (
<EuiNotificationBadge
data-test-subj={`embeddablePanelNotification-${notification.id}`}
key={notification.id}
style={{ marginTop: '4px', marginRight: '4px' }}
onClick={() => notification.execute({ ...context, trigger: panelNotificationTrigger })}
>
{notification.getDisplayName({ ...context, trigger: panelNotificationTrigger })}
</EuiNotificationBadge>
);
if (notification.getDisplayNameTooltip) {
const tooltip = notification.getDisplayNameTooltip({
...context,
trigger: panelNotificationTrigger,
});
if (tooltip) {
badge = (
<EuiToolTip position="top" delay="regular" content={tooltip} key={notification.id}>
{badge}
</EuiToolTip>
);
}
}
return badge;
});
}

@joshuarrrr
Copy link
Member

I'd like to try to re-frame this in more general terms. It seems like the goal here is to have some sort of visual indication (badge) on the panel that indicates (new? unread?) events that are linked to that embeddable. So it seems like we'd be building a new event notification and management interface. At a global level, it would decorate the context menu icon with a notification dot, and add a new UI action to view these events.

But there are still a number of unanswered questions:

  1. Is the View Events UI action a generic action available as part of any embeddable, or is it a property specific to dashboard panels? That determines where the UI action will be registered
  2. Is it sufficient to have a single, shared event listing per panel/embeddable? This is probably preferable as an initial interface than needing to support multiple.
  3. Where/how do users view the individual events? What about past/previously read events?
  4. What's the polling pattern to check for new updates? Is that determined by the plugin registration, or on a fixed schedule, or only when loading/updating the embeddable? Depending on the answer here, we may need to be particularly careful about potential performance degredations, or provide users with a way to turn off or disable updates.

I guess my perspective here is that any event notification system we want to add to dashboard panels should be a standalone pluggable system, not a bunch of isolated UI elements that must be individually toggled and specified.

@joshuarrrr
Copy link
Member

Closing this issue, as it's no longer required for the updated UX guidelines, and we don't have clear enough user stories to justify adding this feature yet. @lezzago feel free to reopen if I missed something here.

@joshuarrrr joshuarrrr added wontfix This will not be worked on invalid This doesn't seem right and removed discuss needs research labels Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request invalid This doesn't seem right wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants