[Observability RAC] Alerts table post-EuiDataGrid style updates

[andrew-goldstein]

[Observability RAC] Alerts table post-EuiDataGrid style updates

This PR updates styles in the Observability Alerts table, as a follow-up to the TGrid migrating to use EuiDataGrid for rendering, and this PR, which improved the alerts table columns.

• The Reason column uses up the remaining width, a follow-up task from [Observability RAC] Improve alerts table columns #105446
  • This task was originally tracked by [Observability RAC] Improve alerts table columns #105227
• Increased the font weight and vertically aligned the Actions header with the other columns
• Removed the Status column (EDIT: we won't remove this, per a discussion w/ UX)
• Increased the width of the Triggered column
• Renamed the Duration column to Alert duration (EDIT: we won't rename this, per a discussion w/ UX)
• Eliminated the gap between actions
• Added truncation to the Reason column

Before

After

Desk testing

• To desk test the Observability > Alerts page, add the following settings to config/kibana.dev.yml:

xpack.observability.unsafe.cases.enabled: true
xpack.observability.unsafe.alertingExperience.enabled: true
xpack.ruleRegistry.write.enabled: true

cc @mdefazio

[elasticmachine]

Pinging @elastic/logs-metrics-ui (Team:logs-metrics-ui)

[andrew-goldstein]

columns that don't have an initialWidth automatically use up the remaining width

[mgiota]

This looks good in large screen resolutions. In smaller resolutions (even at 1200px) the reason text gets cut off.

Since the table is not responsive, could we possibly add an overflow: scroll to the EventsTBody as shown here 8bb0f07#diff-d98503225da4a83c1a846a2185b182229b868fd580d0ef19182ff235e5f98a41R206? At least there will be a horizontal scrollbar and user will be able to read the text.

Unless there is a responsive attribute that we could add to the EuiDataGrid Table that automatically makes the table responsive? Are you aware of something like this?

[andrew-goldstein]

The table itself is responsive, and after removing initialWidth, the Reason column expands to fill with width of the browser, per the animated gif below:

The ... truncation was not being applied to the link text at smaller resolutions due to a subtle change in rendering behavior when EuiLink is used with an onClick prop:

EuiDataGrid automatically applies text truncation to the cell, but EuiLink automatically renders links via a button instead of an a when an onClick prop is passed to EuiLink.

I'm pushing a commit with the following change in render_cell_value.tsx, but I won't merge it until I hear back from @chandlerprall to see if there's another way to achieve this without passing an empty href to EuiLInk:

          // NOTE: EuiLink automatically renders links using a <button>
          // instead of an <a> when an `onClick` prop is provided, but this
          // breaks text-truncation in `EuiDataGrid`, because (per the HTML
          // spec), buttons are *always* rendered as `inline-block`, even if
          // `display` is overridden. Passing an empty `href` prop forces
          // `EuiLink` to render the link as an (inline) <a>, which enables
          // text truncation, but requires overriding the linter warning below:
          // eslint-disable-next-line @elastic/eui/href-or-on-click
          <EuiLink href="" onClick={() => setFlyoutAlert && setFlyoutAlert(alert)}>
            {alert.reason}
          </EuiLink>

[andrew-goldstein]

I'm pushing a commit with the following change in render_cell_value.tsx, but I won't merge it until I hear back from @chandlerprall to see if there's another way to achieve this without passing an empty href to EuiLInk:

Per an out-of-band conversation with @chandlerprall, we decided to merge this PR as-is, and follow-up re: the empty href on EuiLink in a separate PR, if necessary.

[andrew-goldstein]

Post-merge update: this workaround is valid for now, and tracked by the following EUI issue: elastic/eui#4990

Details from the issue above:

Apparently by HTML's design / spec, text inside a button element will not receive a parent's text-overflow: ellipsis - https://stackoverflow.com/questions/9905409/is-it-possible-to-use-text-overflow-ellipsis-on-a-button-element

[mgiota]

It looks nice and fixes the issues according to the specs! There are a few small things I noticed:

• As part of this PR [Observability RAC] Improve alerts table columns [Observability RAC] Improve alerts table columns #105446, duration content was aligned to the right. As a side-effect when I hover on the item in the duration column, your nicely left-aligned content gets aligned to the right. Personally I like the content aligned to the left. Since in the design screenshots for this ticket the duration column is left aligned, I guess you could delete the change I made in the render_cell_value file
  . It would be great if we could have some design input here.

• In medium-large screen resolutions and below (1200px and below) the text in the reason column gets cut off. You can have a look at my comment above

Further improvements
(we could tackle them in separate tickets?)

• Can we make the focus state of the header elements similar to how they look in the security alerts table? Here the border is blue, whereas in the security table it has a light grey background with a grey border

• Let's delete the borders from the actions cells and add a custom css rule to display a border only on last element

[andrew-goldstein]

Thanks for your feedback @mgiota! 😄

Regarding the alignment of duration:

It looks nice and fixes the issues according to the specs! There are a few small things I noticed:

• As part of this PR [Observability RAC] Improve alerts table columns [Observability RAC] Improve alerts table columns #105446, duration content was aligned to the right. As a side-effect when I hover on the item in the duration column, your nicely left-aligned content gets aligned to the right. Personally I like the content aligned to the left. Since in the design screenshots for this ticket the duration column is left aligned, I guess you could delete the change I made in the render_cell_value file
  . It would be great if we could have some design input here.

I agree, and also like the left-aligned duration shown in the mock below:

Above: the mock from https://github.com/elastic/security-team/issues/1299 , with left-aligned duration

We need to troubleshoot the behavior you're seeing where the style is applied on-hover, but as a temporary workaround, I think it makes sense to (for now) left-align it like the mock. @mdefazio are you OK with left-aligned duration for now?

• In medium-large screen resolutions and below (1200px and below) the text in the reason column gets cut off. You can have a look at my comment above

I see what you mean; the column now expands to take up the remaining width, but now it needs truncation. I'll push an update to this PR to add truncation.

Further improvements
(we could tackle them in separate tickets?)

• Can we make the focus state of the header elements similar to how they look in the security alerts table? Here the border is blue, whereas in the security table it has a light grey background with a grey border

• Let's delete the borders from the actions cells and add a custom css rule to display a border only on last element

The legacy TGrid is styled like suggestions above, but the new styles are EuiDataGrid's defaults, as shown in the following animated gif of the EuiDataGrid interactive docs:

To keep things moving forward, I'm wondering if it would make sense to make changes to the header-cell selection in a follow-up PR. @mdefazio do you have any thoughts on overriding the EuiDataGrid defaults to align with the previous styling?

[mgiota]

@andrew-goldstein I totally agree, let's keep things moving forward and we address the changes to the header-cell selection in another PR.

Regarding left alignment issue, you can remove the changes I've done in a previous PR https://github.com/elastic/kibana/pull/105446/files#diff-8c01a3c597461ead53f52ba5028576fc4465fcea366eb4526a60c676bc899912R62.

[andrew-goldstein]

@andrew-goldstein I totally agree, let's keep things moving forward and we address the changes to the header-cell selection in another PR.

Regarding left alignment issue, you can remove the changes I've done in a previous PR https://github.com/elastic/kibana/pull/105446/files#diff-8c01a3c597461ead53f52ba5028576fc4465fcea366eb4526a60c676bc899912R62.

👍 I'll remove it since we're all in agreement the left alignment looks good.

Thanks to @XavierM for helping to debug the alignment / hover issue, which is reproducible using (just) the EuiDataGrid code sandbox example: https://codesandbox.io/s/holy-lake-yf9bt

The good news is this issue appears to be fixed in EUI v36.0.0, so we just need to wait for Kibana's package.json to bump from

"@elastic/eui": "35.0.0",

to 36.0.0

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 8bb4f9b
• Storybooks Preview
• Documentation Changes

Metrics [docs]

Async chunks

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

id	before	after	diff
observability	490.7KB	490.7KB	-60.0B

History

• 💚 Build #140514 succeeded 4875ac2674bbef71f2ee7aedee2dcd2f87508be0
• 💛 Build #139934 was flaky 37f8bb76b1d94e6b3596a9301f92a382cb184b3a

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

cc @andrew-goldstein

[kibanamachine]

💚 Backport successful

Status	Branch	Result
✅	7.x

This backport PR will be merged automatically after passing CI.