[Monitoring] Address some UI regressions with shard allocation #29757
Conversation
chrisronline
added
review
v7.0.0
Team:Monitoring
|
Pinging @elastic/stack-monitoring |
|
Related to #20772 |
💚 Build Succeeded |
| const classes = calculateClass(shard); | ||
| const color = getColor(classes); | ||
| const classification = classes + ' ' + shard.shard; | ||
|
|
||
| let shardUi = ( | ||
| <EuiTextColor color={color}> |
There was a problem hiding this comment.
Is this color code working? In the screenshot, the color doesn't match any of the badge colors. Perhaps the shardUi should be a badge instead of a dark block of text with a color that may not show well on that background?
💔 Build Failed |
|
@chrisronline not related, but perhaps something you can tackle here... I just added a note in another issue #29100 (comment) about a hex value in |
|
@chrisronline I see that this PR is failing for the same reason as a backport PR I have open (for the design changes that were merged to master) - #29764. My original PR passed, but these are not, and that particular test appears to go into a |
|
@ryankeairns I removed the hex and am looking into the test failures. @pickypg Good suggestion. I'm not sure the colors are the best. Any thought? |
💚 Build Succeeded |
|
retest |
💚 Build Succeeded |
|
@ryankeairns The test looks flaky and I'm not sure why. This PR now passes and I think maybe the UI changes we made removed the flakliness. Once this is merged and backported, maybe you can merge 6.x into your backport branch and try CI again |
|
@chrisronline I'll give that a shot, thank you! |
There was a problem hiding this comment.
LGTM. I don't have any good suggestions for colors, and I suspect our past dependence on them bites us for colorblind users anyway. We should probably look at replacing this entire shard "view" with something else and preferably as its own table per node / index so that it can take up more space and show more information (like symbols + shard ID rather than just colors + shard ID).
chrisronline
deleted the
monitoring/fix_shard_allocation_ui_regressions
branch
|
@chrisronline do you have a backport open for this one? That will get my own backport PR unblocked by the flaky test. Thanks! |
|
@ryankeairns Just opened! #29993 |
|
Backport: |
|
This never made it into 6.6.1 FYI |
|
6.6 backport: 7660ee0 |
|
Is this going to be fixed in 6.6.2? How do I uptake the backport? |
|
@sinzin91 This is slated for the next patch release of 6.6. Keep an eye out for our next release |
Fixes #29753
Fixes #20772
During the EUI migration, we had some UI regressions in the Elasticsearch shard allocation UI.
This PR fixes those. See the issue for more details.
TODO