[Logs UI] Allow for jumping to the previous and next highlight #40010
Conversation
…nto 22916-log-highlighting
…nto 22916-log-highlighting
* Show a highlight is active * Focus input when menu is opened * Icon for highlights menu * Debounce onChange handler * Loading state for highlights menu * Neaten up Provider
* Ensure width of popover stays consistent * Add a button to clear highlights
While a `phrase_prefix` query feels more natural, it doesn't work on `keyword` type fields. Using just a `phrase` query keeps the highlighting consistent between text and keyword type fields.
I want to be careful of sunk cost thinking getting in the way of our decision making, but this is fair and would obviously be frustrating. Looking back at #22916, I admit I didn't have a full sense of how this worked outside of "search for term, instances are highlighted", which also happens to sound like a pretty good first iteration of the feature. I'll try to be better about asking for clarification earlier. 👍 With that in mind, what do we think are the minimum set of features that make highlighting useful in the log UI today? That way we can focus on those and then regroup to think about all of the various coloring issues as a whole. If we think next/prev is on that list, then I'm in favor of "showing the currently highlighted entry slightly differently for now". Related: the HSL modification of the highlight color already gives it some contrast problems, especially in dark mode:
The brighter color here is the full EUI accent color, and the more muted version has been HSL-desaturated, etc. to make it less intense based on earlier design feedback. If we revert the standard highlight color back to the full EUI accent color, and then drop the opacity of the others, that might solve all of these issues in one swoop. (Q: Would we immediately enter into this state as soon as you highlight a term, where the "active" one is highlighted differently? Or would it only go into that state when you interact with next/prev?) |
totally agree, thanks for calling me out on that 😉 Maybe the highlighted terms would only be outlines except for the active one, which would have a filled background? Then we wouldn't have to deal with multiple hues and can just pick one with a good readability. |
💔 Build Failed |
|
@jasonrhodes @Kerry350 I'm trying to color the currently active highlight differently... the color and contrast fiddling is tying my brain into a knot - this is what I have:
(I'm not a fan of the pink accent TBH, but that's what EUI uses 🤷♂️) It's based on the accent color with a bit of |
|
@weltenwort phew, contrast math mania! I like how it looks, I'm going to test out how it works right now but it looks like it'll be much clearer. 👏 Could we just opt for hard-coding the text-color values and assume that we will have a bright color for the active and a duller one for the inactive, so we'll always need the opposite text color? I can imagine those brain knots you mention coming up again and again if we ever need to see what this is flow is doing and I'd prefer to see if we can start simple. |
|
I introduced utils like Let me know if it looks ok for you. I'm taking a look at the failing test in the meantime. |
Much clearer what's going on now, good change 👍 |
There was a problem hiding this comment.
Let's keep an eye on the usage of the color/lightness utility and if we are able to solve colors in the Logs UI without needing those kinds of utils, let's just remember to retire it rather than leave it to use "one day". Hopefully those kinds of utils, if needed, will come from EUI eventually.
This looks great, and the active color looks good in light and dark modes. Thanks @Kerry350 and @weltenwort !
|
Just added the same effect to the "field" type column too. I can't reproduce the previous test failures locally... let's see if try occur again. |
|
🤞 |
💔 Build Failed |
|
I can only reproduce this failure on the pinned Elasticsearch snapshot Kibana CI currently uses. On more recent snapshots this failure doesn't occur. The pinning of the version is tracked in #41538. |
|
Some more background: The error suggests the |
|
bonus background: My hypothesis about the sudden appearance and disappearance of this test failure is that apache/lucene-solr@57eb779 seems to have introduced a rewriting step that flattens nested |
💚 Build Succeeded |
…ic#40010) This PR adds buttons to the highlighting popover that allow for jumping to the previous and next highlights. The intended user experience should be close to the "find" experience of many text editors.
Backports the following commits to 7.x: - [Logs UI] Allow for jumping to the previous and next highlight (#40010)
Summary
This PR adds buttons to the highlighting popover that allow for jumping to the previous and next highlights. The intended user experience should be close to the "find" experience of many text editors.
Preview
Known Limitations
Jumping to the initial highlight is inaccurate in some situations. This problem is not specific to this implementation, but relates to [Logs UI] Jumping to a new time target outside the range of loaded entries is inaccurate #39944.(fixed in [Logs UI] Fix inaccuracy when jumping to a faraway time target #40303)Checklist
Documentation was added for features that require explanation or tutorials(We assume the feature is a self-explanatory addition.)