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.

Sign up for GitHub

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

Jump to bottom

[Response Ops] Alerts table issues #171738

Closed
mgiota opened this issue Nov 22, 2023 · 6 comments · Fixed by #186978
Closed

[Response Ops] Alerts table issues #171738

mgiota opened this issue Nov 22, 2023 · 6 comments · Fixed by #186978
Assignees
@umbopepato
Labels
bug Fixes for quality problems that affect the customer experience Feature:SLO Team:obs-ux-management Observability Management User Experience Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)

Comments

@mgiota
Copy link
Contributor

mgiota commented Nov 22, 2023 •
edited
Loading

🍒 Summary

As part of implementing the Slo alerts Embeddable widget, we used the shared Alerts table from response Ops team. We noticed 2 issues, for which we would need support from the Response Ops team.

  • There is an issue with the loading indicator (not always reproducible), which sometimes goes away and sometimes it doesn't, unless you refresh the browser.
  • The rows jump on hover
Screen.Recording.2023-11-22.at.14.12.08.mov

✅ Acceptance criteria

  • The loading indicator should go away when the data/alerts finished loading
  • The rows shouldn't jump on hover

Observations

This change seemed to fix the issue, but once I switched to a custom alerts table configuration, loading issue not going away happened again. Not consistently though, a race condition. Custom alerts configuration might be irrelevant though, just a simple observation.
@mgiota mgiota added bug Fixes for quality problems that affect the customer experience Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams) Team:obs-ux-management Observability Management User Experience Team labels Nov 22, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/response-ops (Team:ResponseOps)

@elasticmachine
Copy link
Contributor

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

@cnasikas cnasikas changed the title [Response Ops] [Response Ops] Alerts table issues Nov 22, 2023
@mgiota mgiota mentioned this issue Dec 4, 2023
Merged
@XavierM XavierM removed their assignment Jan 23, 2024
@umbopepato
Copy link
Member

After investigating this in #175119 as well, I've found that the loading indicator issue happens when the specified sort option refers to a column that is not specified in the columns array. In this case when EuiDataGrid updates, it resets the sort property seeing as the corresponding column is not present in the table.

I'll make sure to add a JSDoc highlighting this aspect in the alerts table configuration options typing. If the intention is to provide a default/fallback sorting option I guess we could do this at AlertsTable-level or provide a dedicated prop.

@umbopepato umbopepato mentioned this issue Jan 24, 2024
Closed
@umbopepato umbopepato mentioned this issue Feb 2, 2024
Merged
umbopepato added a commit that referenced this issue Feb 13, 2024
@umbopepato
[RAM] Fix AlertsTable sorting with inactive columns and default sort …
dbf5f97 
…direction (#176172)

## Summary

Separates the sorting configuration state by visible and invisible
columns, allowing consumers of the AlertsTable to specify
`SortCombinations` that include fields not currently visible in the
table. Changes the default sorting direction to descending.

Closes #175466
Refs #171738
CoenWarmer pushed a commit to CoenWarmer/kibana that referenced this issue Feb 15, 2024
@umbopepato @CoenWarmer
[RAM] Fix AlertsTable sorting with inactive columns and default sort …
1e7dcd5 
…direction (elastic#176172)

## Summary

Separates the sorting configuration state by visible and invisible
columns, allowing consumers of the AlertsTable to specify
`SortCombinations` that include fields not currently visible in the
table. Changes the default sorting direction to descending.

Closes elastic#175466
Refs elastic#171738
fkanout pushed a commit to fkanout/kibana that referenced this issue Mar 4, 2024
@umbopepato @fkanout
[RAM] Fix AlertsTable sorting with inactive columns and default sort …
67551a2 
…direction (elastic#176172)

## Summary

Separates the sorting configuration state by visible and invisible
columns, allowing consumers of the AlertsTable to specify
`SortCombinations` that include fields not currently visible in the
table. Changes the default sorting direction to descending.

Closes elastic#175466
Refs elastic#171738
@umbopepato
Copy link
Member

umbopepato commented Mar 6, 2024 •
edited
Loading

@mgiota #176172 should have fixed the loading issue (now you can specify sorting fields independently of the corresponding column being visible).
Are you still experiencing the rows jumping issue?

@mgiota
Copy link
Contributor Author

mgiota commented Mar 7, 2024

@umbopepato No I don't experience any rows jumping issue anymore. But I think I still see the loading issue from time to time.

@mgiota
Copy link
Contributor Author

mgiota commented Mar 8, 2024

Screen.Recording.2024-03-08.at.01.01.08.mov

@XavierM XavierM moved this from In Review to On Hold in AppEx: ResponseOps - Rules & Alerts Management Mar 20, 2024
@umbopepato umbopepato mentioned this issue Jul 10, 2024
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@umbopepato umbopepato

Labels
bug Fixes for quality problems that affect the customer experience Feature:SLO Team:obs-ux-management Observability Management User Experience Team Team:ResponseOps Label for the ResponseOps team (formerly the Cases and Alerting teams)
Projects
No open projects
AppEx: ResponseOps - Rules & Alerts Management
Status: On Hold
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[ResponseOps][Alerts] Migrate alerts fetching to TanStack Query umbopepato/kibana
4 participants
@XavierM @mgiota @elasticmachine @umbopepato