Update alert type selection layout to rows instead of grid

[mdefazio]

Summary

Changes the alert type selection screen to use rows with search bar and filter by solutions instead of the KeypadMenuItem for better scanning and alignment.

Resolves #67139

[elasticmachine]

Pinging @elastic/kibana-alerting-services (Team:Alerting Services)

[mdefazio]

I have a placeholder for a short description on each one of these. If we don't think we can get those together, then perhaps we can just move towards using a combo box instead as the component. Though I do think eventually we will the descriptions regardless the component we use.

[YulNaumenko]

Code LGTM. But from the UX prospective I think it might be better to have a combobox with a search and maybe grouping by an AlertType producer.
As we discussed before, view with cards is looking pretty, but till we meet the big number of alert types. As an alternative it could be an endless scroll with the search, but I believe that combobox list with the search tool is the best solution.

[andreadelrio]

Wondering if it would make more sense to keep the EuiKeyPadMenuItem items and adjust height/alignment until we're able to move "alert type selection" to its own screen? I don't think this layout works well for the current flyout, lots of scrolling needed to find the type furthest down the list. If we do keep this new layout I agree that we need some sort of search functionality. How would using a combobox look?

[mdefazio]

Wondering if it would make more sense to keep the EuiKeyPadMenuItem items and adjust height/alignment until we're able to move "alert type selection" to its own screen? I don't think this layout works well for the current flyout, lots of scrolling needed to find the type furthest down the list. If we do keep this new layout I agree that we need some sort of search functionality. How would using a combobox look?

I think we should move toward getting the descriptions in there. I know the longer scrolling isn't ideal (perhaps we shrink the size/spacing a bit), and I'm all for having a search option as well. But I think the trade-off for having descriptions would be more helpful. The combobox would need a click to open it, so I think it would work well enough with the current layout that includes the name/tags/schedule above. I will try switching this so we can see how it works.

[YulNaumenko]

New ComboBox looks great and works as expected! 👍
I have a few changes suggestions in the code to simplify it a little. One console issue come up when I tested it locally:
It might be related to the react state update of alertTypeModel and depending visibility of the ComboBox component. I've tested how it works without the condition for hiding it and error disappeared - I can help you to figure out the problem.

[andreadelrio]

• Can we add a heading to the section? Alert trigger or Alert type perhaps? It would be replaced by the actual alert type name when it is selected.
• I'd add a default value to the combobox or a placeholder
• Why does the Actions section only show up once they've selected the alert type? Isn't that section common to all alert types? I think the Actions section should show up at all times, just like the first section.

[ymao1]

It's looking great! A few questions (with a screenshot for reference)

• The alert types are alphabetized by solution name (which I like) but the Filter by solutions dropdown is not. Can we alphabetized the dropdown?

• The Filter by solutions dropdown contains solutions that don't have associated alerts (in the screenshot above, Security, Stack Monitoring and APM and User Experience will filter the list down to 0). Maybe this is because I don't have the right configs that I don't have alert types for these solutions? But would it make sense to have the solutions dropdown only contain solutions that have alert types?

• In the Option links to styles and js #1 mockup from the above comment: Update alert type selection layout to rows instead of grid #73665 (comment), the both the name of the solution group and the name of the alert are left-aligned, but in the PR, the alert types are indented. Maybe you've already had this discussion, but I think the left-aligned look is better for scanning.

• Not sure why but my second Uptime alert type (bottom of the screenshot) shows the alert id (xpack.uptime.alerts.monitorStatus) instead of the alert name

[YulNaumenko]

It's looking great! A few questions (with a screenshot for reference)

• The alert types are alphabetized by solution name (which I like) but the Filter by solutions dropdown is not. Can we alphabetized the dropdown?
• The Filter by solutions dropdown contains solutions that don't have associated alerts (in the screenshot above, Security, Stack Monitoring and APM and User Experience will filter the list down to 0). Maybe this is because I don't have the right configs that I don't have alert types for these solutions? But would it make sense to have the solutions dropdown only contain solutions that have alert types?
• In the Option links to styles and js #1 mockup from the above comment: #73665 (comment), the both the name of the solution group and the name of the alert are left-aligned, but in the PR, the alert types are indented. Maybe you've already had this discussion, but I think the left-aligned look is better for scanning.
• Not sure why but my second Uptime alert type (bottom of the screenshot) shows the alert id (xpack.uptime.alerts.monitorStatus) instead of the alert name

Thank you for a feedback!

• The alert types are alphabetized by solution name (which I like) but the Filter by solutions dropdown is not. Can we alphabetized the dropdown?

I will fix the sorting, good catch!

• The Filter by solutions dropdown contains solutions that don't have associated alerts (in the screenshot above, Security, Stack Monitoring and APM and User Experience will filter the list down to 0). Maybe this is because I don't have the right configs that I don't have alert types for these solutions? But would it make sense to have the solutions dropdown only contain solutions that have alert types?

Basically this solutions has alerts, but those are not registered in the alerting UI. So I agree, that it doesn't make any sense to have a filter by non visible alerting producers

• In the Option links to styles and js #1 mockup from the above comment: #73665 (comment), the both the name of the solution group and the name of the alert are left-aligned, but in the PR, the alert types are indented. Maybe you've already had this discussion, but I think the left-aligned look is better for scanning.

I will rely on @mdefazio to fix the UI alignments - he already has something not pushed yet.

• Not sure why but my second Uptime alert type (bottom of the screenshot) shows the alert id (xpack.uptime.alerts.monitorStatus) instead of the alert name

Yeah, it is because they use JSX element instead of text, but agree that I need to handle this as well

The original plan and implementation has changed quite a bit.

[YulNaumenko]

It's looking great! A few questions (with a screenshot for reference)

• The alert types are alphabetized by solution name (which I like) but the Filter by solutions dropdown is not. Can we alphabetized the dropdown?
• The Filter by solutions dropdown contains solutions that don't have associated alerts (in the screenshot above, Security, Stack Monitoring and APM and User Experience will filter the list down to 0). Maybe this is because I don't have the right configs that I don't have alert types for these solutions? But would it make sense to have the solutions dropdown only contain solutions that have alert types?
• In the Option links to styles and js #1 mockup from the above comment: #73665 (comment), the both the name of the solution group and the name of the alert are left-aligned, but in the PR, the alert types are indented. Maybe you've already had this discussion, but I think the left-aligned look is better for scanning.
• Not sure why but my second Uptime alert type (bottom of the screenshot) shows the alert id (xpack.uptime.alerts.monitorStatus) instead of the alert name

@ymao1 I've addressed changes that you mentioned here, ready for review 👍

[ymao1]

Would it be simpler here to reduce availableAlertTypesResult instead of alertTypesResult since that is already filtered down to the available?

const solutionsResult2 = availableAlertTypesResult.reduce(
    (result: Array<{ id: string; title: string }>, alertTypeItem) => {
        if (!result.find((solution) => solution.id === alertTypeItem.alertType.producer)) {
            result.push({
                id: alertTypeItem.alertType.producer,
                title:
                    (kibanaFeatures
                        ? getProducerFeatureName(alertTypeItem.alertType.producer, kibanaFeatures)
                        : capitalize(alertTypeItem.alertType.producer)) ?? capitalize(alertTypeItem.alertType.producer),
            });
        }
        return result;
    },
    []
);

[ymao1]

One more alphabetization nit:

It looks like the dropdown is alphabetized by producer name and the list is alphabetized by producer id. In this case, the producer Metrics has the id infrastructure which causes it to show up above Logs in the list and below Logs in the dropdown.

[MichaelMarcialis]

Hey, @mdefazio. This is looking great and it works as advertised. I did leave a handful of comments that I would consider low priority nitpicks, but have a look and let me know your thoughts.

[MichaelMarcialis]

Rather than showing the number of actively selected filters, I believe the correct pattern for prop numFilters is to instead show the total number of available filters. For example, with no filters active, the EuiFilterButton should show the total number of filters available to choose from.

[YulNaumenko]

We have this behavior the same across Alerting Plugin - filters on Alerts list. So maybe should address this as a separate issue and do the same change for all filters. Is it OK?

[MichaelMarcialis]

Sounds good. Let me know if you'd like me to make the issue.

[YulNaumenko]

@MichaelMarcialis, please open the issue. Thank you!

[ymao1]

Looks great!

[MichaelMarcialis]

Thanks for acknowledging my little nitpicks. Approving.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: a197fcf

Metrics [docs]

@kbn/optimizer bundle module count

id	before	after	diff
triggersActionsUi	287	297	+10

async chunks size

id	before	after	diff
triggersActionsUi	1.5MB	1.5MB	+12.7KB

distributable file count

id	before	after	diff
default	42766	42767	+1

History

• 💚 Build #86555 succeeded e6ced0d
• 💚 Build #86544 succeeded ae9faba
• 💔 Build #86320 failed 9d5595d
• 💔 Build #85940 failed 489ee44
• 💚 Build #85731 succeeded 850a030

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