fix(ui): add a tooltip for commonly truncated fields in the events pane #5062
Conversation
dcherman
force-pushed
the
fix/events-panel-usability
branch
4 times, most recently
from
c6b199d
to
c70a05c
Compare
|
Alternatively, we could also just always add tooltips regardless of whether or not the text is truncated. |
|
I think it would be fine to mouse over and get a pop-up, I think @simster7 added something similar recently |
It's not uncommon for the involved object and/or message to be longer than can be displayed in the events pane, and finding out what the full string is has often involved going into the inspector to look at the contents of the element. Adding a tooltip on these will help mitigate that a bit. Signed-off-by: Daniel Herman <dherman@factset.com>
dcherman
force-pushed
the
fix/events-panel-usability
branch
from
c70a05c
to
7c89fd6
Compare
|
All the time or conditionally? If we want it all the time, I'll swap it to that |
| cancel(); | ||
|
|
||
| timer = window.setTimeout(() => { | ||
| fn(...args); |
There was a problem hiding this comment.
calculateTooltips doesn't take parameters?
There was a problem hiding this comment.
calculateTooltips itself doesn't, but this is a general purpose debounce function similar to what you might find in lodash and/or underscore - it's very useful for deferring work until some frequently occurring event is no longer occuring (in this case, window resize)
https://lodash.com/docs/#debounce
This function supports functions with any arity
| const debouncedFn = (...args: Parameters<T>) => { | ||
| cancel(); | ||
|
|
||
| timer = window.setTimeout(() => { |
There was a problem hiding this comment.
See the comment discussing what debounce does (I can add comments to the code as well) - essentially the function that's passed in will not be invoked until the debounced function has not been invoked for N milliseconds
| if (col.scrollWidth > col.clientWidth) { | ||
| col.title = col.textContent; | ||
| } else { | ||
| col.title = ''; |
There was a problem hiding this comment.
Should this be ... instead? Or something to indicate we're cutting off text?
There was a problem hiding this comment.
title is just the tooltip that will show up when you hover over the element with your mouse. The browser itself takes care of truncating with ... when necessary. Setting this to empty basically just disables the tooltip when the full text is visible in your window
| const [debouncedCalculateTooltips, cleanup] = debounce(calculateTooltips, 1000); | ||
|
|
||
| window.addEventListener('resize', debouncedCalculateTooltips); | ||
| calculateTooltips(); |
There was a problem hiding this comment.
Seems like this is called twice? One in debounce and one here?
There was a problem hiding this comment.
Correct - this is immediately called in order to set up tooltips based on your current window size, then we call it in a debounced event handler on window resize to recalculate whether or not the tooltips need to be displayed. We debounce it in order to avoid layout thrashing since we access properties like clientWidth and scrollWidth in the function.
|
Will add some screenshots / GIFS later today |
|
Here's a quick GIF demoing the transition from no tooltip, to having a tooltip, to not having one again.
|
It's not uncommon for the involved object and/or message to be longer
than can be displayed in the events pane, and finding out what the full
string is has often involved going into the inspector to look at the contents
of the element.
Adding a tooltip on these will help mitigate that a bit.
Signed-off-by: Daniel Herman dherman@factset.com