[ObsUx][Infra] Add collapsible sections in the overview tab

[jennypavlova]

Closes #175989

Summary

This PR is a follow-up to #175558. It adds the active alerts count next to the alert section title (this will happen after the alerts widget is loaded) following the rules:

Default behaviour
No alerts at all ==> Collapse and say 'No active alerts'
No active alerts ==> Collapse and say 'No active alerts'
Active alerts ==> Expand fully

Collapsed
No alerts at all ==> Say 'No active alerts'
No active alerts ==> Say 'No active alerts'
Active alerts ==> say "X active alerts"

It adds a change in the AlertSummaryWidget to make it possible to get the alerts count after the widget is loaded using a new prop.

This PR also changes the alerts tab active alert count badge color on the hosts view to keep it consistent:

Before	After

Testing

• Open hosts view and select a host with active alerts (flyout or full page)
  • The alerts section should be expanded showing the alerts widget
    fd1a21035a5f)
  • Collapse the alerts section by clicking on the title or the button:
    
• Open hosts view and select a host without active alerts (flyout or full page)
  • The alerts section should be collapsed showing the message 'No active alerts'
    

alerts_section_with_alerts_count.mov

[apmmachine]

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

• /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
• /oblt-deploy-serverless : Deploy a serverless Kibana instance using the Observability test environments.
• run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

[jennypavlova]

/ci

[elasticmachine]

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

[crespocarlos]

Hey @jennypavlova , thanks for the adjustments. I have a couple of suggestions/questions:

I know we didn't have any designs for the message next to the alert title, but I'm wondering if we could use badges to display the alert count, to make it consistent with how we're going to display it in the Hosts Table:

https://docs.google.com/document/d/1XHcBdDjfJfA8sT1H1vHwG16qKxQCVbQ_Cy4zI3_zHaA/edit#heading=h.php0dgdhwjlp

When there is no active alerts, I wonder if we should still allow users to try to expand the section

no_active_alerts.mov

[jennypavlova]

I know we didn't have any designs for the message next to the alert title, but I'm wondering if we could use badges to display the alert count, to make it consistent with how we're going to display it in the Hosts Table

That's a good idea, thanks for sharing that. I tried to keep it simple for now before we get the design ready and discussed those messages with @roshan-elastic as a first iteration while we were waiting for the design. We can have either a badge (like those in the table) or the same style as the widget - I can try both and see how it looks :)

When there is no active alerts, I wonder if we should still allow users to try to expand the section

I know what you mean here - initially I showed the same "No alerts" message there but I don't think it's useful with the default closed section. Another case we miss here is when for example there are 5 recovered alerts the user should be still able to see the section there (even if there are no active alerts), so in short, we should still have the option to expand the section.
There are 2 cases when we have no alerts (any alerts) - when we don't have any rules or when the host is "healthy": If you ask me we can have something visual there that no active alerts are present or a prompt with a button to link to rule creation (in case of no rules) or both somehow - but IMO this should be handled in the alerts summary widget

[jennypavlova]

@roshan-elastic @crespocarlos So question about the active alerts message based on the discussion above and the alerts designs here I tried 2 versions of the closed section message with active alerts:

• Same as the widget:
  

• Same as table (badge):
  

• Note: The current version is just text:
  

TBH I like the red color there but I can't decide which one fits better 😅 Wdyt?

BTW off-topic: but we can maybe also add an issue to convert the pink alert badge to red inside the hosts view tab so it will be also consistent with the design of the alerts feature (of course, if we are not trying to avoid red color for the specific case).

[kpatticha]

do we need typeof alertsCount?.activeAlertCount === 'number' ?

[jennypavlova]

We have the type number | undefined so the first check is to make sure that it's a number and not undefined (Typescript will complain that it can be undefined if we remove the check) Do you think there is something wrong with checking that? We can also change it to alertsCount?.activeAlertCount && ... but I think checking the type is easy to understand and helps validate the activeAlertCount value type I think.

[crespocarlos]

suggestion: you could do this

const { activeAlertCount = 0 } = alertsCount ?? {};
const hasActiveAlerts = activeAlertCount > 0;
...

[jennypavlova]

Done ✅

[crespocarlos]

Thanks for looking into that @jennypavlova .

I like option 2, with the badge, both in the alerts section title and the alerts tab.

[roshan-elastic]

Hey @jennypavlova - I like the idea of being consistent between the alerts tab and the alerts section for sure.

I think we need UX here as I think it needs a little tweaking. Could we wait for @sileschristian to take a look a this?

[roshan-elastic]

Hey @jennypavlova - I like the idea of being consistent between the alerts tab and the alerts section for sure.

I think we need UX here as I think it needs a little tweaking. Could we wait for @sileschristian to take a look a this?

@jennypavlova @crespocarlos

Actually, given we're nearly at FF, @sileschristian still looks sick so I'd rather get this out and tweak it later.

Looking at APM, can we keep it consistent with how they do it (and we can tweak both if we need to once @sileschristian has had a look)?

Alerts in APM

• Use APM alert style in hosts alert tab
• Go with option Change of links to styles and js #2 in collapsed alert section state (but can you look at the alignment*?)

OK to go with that?

*the alignment looks a bit off

[jennypavlova]

@roshan-elastic @crespocarlos Thank you both!

Regarding the alignment - it was just an example, I didn't really look at the position just the item itself and thanks for spotting that, so I set a central alignment it now:

So I changed both as suggested:

Wdyt?

[roshan-elastic]

Good to go @jennypavlova - we can always update later!

Great work! :-D

[jennypavlova]

@roshan-elastic Thanks for checking that!
BTW I forgot the tooltip from the APM badge in the table and now I changed it to have the same styles:

[jennypavlova]

@crespocarlos Can you please take a look again - I updated the description and the screenshots :)

[crespocarlos]

Thanks for the changes @jennypavlova. Left a couple nits

[crespocarlos]

nit: This prop is not a boolean, we could name it collapsibleStatus, wdyt?

[jennypavlova]

Makes sense, renamed ✅

[crespocarlos]

suggestion: you could do this

const { activeAlertCount = 0 } = alertsCount ?? {};
const hasActiveAlerts = activeAlertCount > 0;
...

[crespocarlos]

LGTM! Thanks @jennypavlova.

[thomheymann]

Code generally looks good. Small suggestion below.

[thomheymann]

I think it should be the responsibility of the consumer to decide wether they want to render this component or not instead of having a component that doesn't render anything when no activeAlertCount prop has been passed in.

What do you think?

[jennypavlova]

Thanks @thomheymann, you are right in general but the idea here is that before we set the activeAlertCount we don't want to display any message and the message should be shown once we have the activeAlertCount - so when it's a number.

[Zacqary]

ResponseOps changes LGTM

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 4747262

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
infra	1422	1423	+1

Async chunks

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

id	before	after	diff
infra	1.3MB	1.3MB	+951.0B
triggersActionsUi	1.4MB	1.4MB	+46.0B
total			+997.0B

History

• 💔 Build #192250 failed 401bde1
• 💚 Build #191726 succeeded 9f9edec
• 💚 Build #191570 succeeded 0c98488
• 💚 Build #191452 succeeded 5e3a34f

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

cc @jennypavlova