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

[ML] DFA: Fix layout of job status in expanded row. #204978

Merged
merged 7 commits into from
Jan 7, 2025

Conversation

walterra
Copy link
Contributor

@walterra walterra commented Dec 19, 2024

Summary

This removes the usage of EuiBetaBadge to display the job status. This EUI component has a semantic meaning to be used to display a "beta", "experimental" or other status of a feature, in this case it was used to render the data frame analytics job status. Instead, the job status now gets rendered in the same way like the other items in the list.

Before:

CleanShot 2024-12-19 at 17 36 44@2x

After:

CleanShot 2025-01-07 at 16 43 26@2x

Checklist

  • Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
  • Unit or functional tests were updated or added to match the most common scenarios
  • This was checked for breaking HTTP API changes, and any breaking changes have been approved by the breaking-change committee. The release_note:breaking label should be applied in these situations.

@walterra walterra added :ml release_note:skip Skip the PR/issue when compiling release notes Feature:Data Frame Analytics ML data frame analytics features v9.0.0 backport:version Backport to applied version labels v8.18.0 labels Dec 19, 2024
@walterra walterra self-assigned this Dec 19, 2024
@walterra walterra requested a review from a team as a code owner December 19, 2024 16:38
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@walterra walterra requested review from rbrtj and joana-cps December 19, 2024 16:39
Copy link
Contributor

@rbrtj rbrtj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM 👍

@joana-cps
Copy link

@walterra I agree to move out of the EuiBetaBadge, but we could use the EUI badge for health status instead of just text. This is a relevant status and in text format it loses visual relevance. Also, the badge is already used on the table row, we should be consistent in both places. Let me know what you think.

@walterra
Copy link
Contributor Author

walterra commented Jan 7, 2025

@joana-cps good idea! I updated with EuiBadge in 3c80c88, also updated the screenshot in the PR description.

Copy link

@joana-cps joana-cps left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the changes @walterra

We might consider add different colors to this badge for different status, but let's leave that for later.

@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] Jest Tests #8 / StepDefinePackagePolicy default API response should display vars coming from package policy

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
ml 4.7MB 4.7MB +161.0B

History

cc @walterra

@walterra walterra merged commit c4371e9 into elastic:main Jan 7, 2025
8 checks passed
@walterra walterra deleted the ml-dfa-fix-expanded-row-status branch January 7, 2025 17:34
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/12656339674

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
## Summary

This removes the usage of `EuiBetaBadge` to display the job status. This
EUI component has a semantic meaning to be used to display a "beta",
"experimental" or other status of a feature, in this case it was used to
render the data frame analytics job status. Instead, the job status now
gets rendered in the same way like the other items in the list.

Before:

![CleanShot 2024-12-19 at 17 36
44@2x](https://github.com/user-attachments/assets/17c80e89-8e67-4400-b431-3a1e4c78d642)

After:

![CleanShot 2025-01-07 at 16 43
26@2x](https://github.com/user-attachments/assets/2240d4a7-fb77-4a5b-bfbd-6efeaeda7383)

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.

(cherry picked from commit c4371e9)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
## Summary

This removes the usage of `EuiBetaBadge` to display the job status. This
EUI component has a semantic meaning to be used to display a "beta",
"experimental" or other status of a feature, in this case it was used to
render the data frame analytics job status. Instead, the job status now
gets rendered in the same way like the other items in the list.

Before:

![CleanShot 2024-12-19 at 17 36
44@2x](https://github.com/user-attachments/assets/17c80e89-8e67-4400-b431-3a1e4c78d642)

After:

![CleanShot 2025-01-07 at 16 43
26@2x](https://github.com/user-attachments/assets/2240d4a7-fb77-4a5b-bfbd-6efeaeda7383)

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
crespocarlos pushed a commit to crespocarlos/kibana that referenced this pull request Jan 8, 2025
## Summary

This removes the usage of `EuiBetaBadge` to display the job status. This
EUI component has a semantic meaning to be used to display a "beta",
"experimental" or other status of a feature, in this case it was used to
render the data frame analytics job status. Instead, the job status now
gets rendered in the same way like the other items in the list.

Before:

![CleanShot 2024-12-19 at 17 36
44@2x](https://github.com/user-attachments/assets/17c80e89-8e67-4400-b431-3a1e4c78d642)

After:

![CleanShot 2025-01-07 at 16 43
26@2x](https://github.com/user-attachments/assets/2240d4a7-fb77-4a5b-bfbd-6efeaeda7383)

### Checklist

- [ ] Any text added follows [EUI's writing
guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses
sentence case text and includes [i18n
support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md)
- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
- [x] This was checked for breaking HTTP API changes, and any breaking
changes have been approved by the breaking-change committee. The
`release_note:breaking` label should be applied in these situations.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels Feature:Data Frame Analytics ML data frame analytics features :ml release_note:skip Skip the PR/issue when compiling release notes v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants