Highlight selected rows#1231
Conversation
ghost
added
the
| } | ||
| else | ||
| { | ||
| return $"log-row-{entry.Severity.ToString().ToLowerInvariant()}"; |
There was a problem hiding this comment.
What is combination of log-row-* & selected-row? Could be weird format change on row when selecting-deselecting depending on severity.
There was a problem hiding this comment.
There is no combination. It looks like selected row takes precedence. It's not ideal, but I want to try it out before passing judgement.
There was a problem hiding this comment.
Yeah having selected override the severity was kind of picking the option I disliked the least. Severity overriding selection seemed worse, and trying to come up with a color combo for severity + selection didn't seem likely to work out. I debated outlining + background, but figured something like that should come from feedback.
There was a problem hiding this comment.
Tried selecting error/warning rows and it is Good Enough. The background color is overwritten but the error/warning icons are still visible.
Coming up with color combos for every situation in light and dark would be a lot of work for not much value.
| } | ||
| else | ||
| { | ||
| return $"log-row-{entry.Severity.ToString().ToLowerInvariant()}"; |
There was a problem hiding this comment.
There is no combination. It looks like selected row takes precedence. It's not ideal, but I want to try it out before passing judgement.
| } | ||
| else | ||
| { | ||
| return (viewModel.Span.SpanId == _span?.SpanId) ? "selected-span" : string.Empty; |
There was a problem hiding this comment.
I don't think the concept of selected-span is still used. There used to be a link on the logs page to a span, and it would be selected. The logs link was removed.
But I can double check and clean up later.
Co-authored-by: James Newton-King <james@newtonking.com>
| } | ||
|
|
||
| private string? GetRowClass(ResourceViewModel resource) | ||
| => string.Equals(resource.Name, SelectedResourceName, StringComparison.Ordinal) ? "selected-row" : null; |
There was a problem hiding this comment.
This would be a candidate for StringComparisons.ResourceName, in future.
|
On it |
|
Thanks, Tim. feel free to ping if you need an approval. |
6 participants
Addresses part of #1081.
This adds some highlighting (using the same color as used for the menu hover to ensure contrast) to the various grid rows when they're selected in a Summary/Details view. Also, at the suggestion of various UI/UX/Accessibility folks I changed the subtext color to not be "ghosted" when hovered/highlighted. That avoids the issue of having to thread the needle of contrast with two foreground colors and two (or three) background colors.
The remainder of #1081 is around how we do selection (full row vs buttons, etc) and this PR doesn't address that.