Fix tooltip bug, truncate componentName to use ellipsis #78
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/mlh-fellowship/scheduling-profiler-prototype/rhfe8zhkz |
|
This is how strings longer than 32 characters look like now for componentName. Set a limit to
|
|
Seems like there's still a bug: For the script URL, could we do whatever's being done for component stacks, i.e. that fading and truncation?
The tooltips also go past the edge of the canvas are hidden. Would putting the overflow:hidden on
|
|
It's really a weird bug, half of the script URLs wrap and the other half don't, even though the URL is exactly the same. Reducing it to 64 seemed to fix the issue but you still encountered it, I'll look again.
They will go past the canvas regardless since they're not a part of the canvas but instead a div on top of the canvas. |
|
I'm also concerned about truncating component names. Some of these components seem to be wrapped in a few higher order functions and it'll be impossible to identify the original component that was wrapped. Although in this screenshot you can see it from the component stack, we may not be able to depend on the presence of component stacks (as Brian mentioned it's currently it's not enabled (facebook/react#19396), and facebook/react#19223 (comment) suggests that we may not even want to have component stacks). I'm not sure how the tooltip width is set/derived, but will increasing the max width of the tooltips make sense and truncate only unreasonably long names?
|
Yeah. It's fine that they go past the canvas. What I was trying to achieve was to prevent the clipping of the areas of the tooltip that are outside the canvas' bounds. |
|
What do you think about this? I could set the trim to longer than 128 characters for component names. This is the weird behavior I meant in my earlier comment, the URL length are the same As it should be: The bug: |
Tooltips would go offscreen because of the way `top` is defined. This commit fixes that root cause by changing CanvasPage to position fixed so that tooltips will be positioned relative to the DOM rather than the CanvasPage div. Undoes some of the changes made in #78 as this commit fixes the root cause of the bug.
* Minor clean up of EventTooltip.js * Fix root cause of #47 and improve tooltip styling Tooltips would go offscreen because of the way `top` is defined. This commit fixes that root cause by changing CanvasPage to position fixed so that tooltips will be positioned relative to the DOM rather than the CanvasPage div. Undoes some of the changes made in #78 as this commit fixes the root cause of the bug. * Disable component name truncation Although this was Brian's recommendation (see #67), I don't think component names will be very long because the source code for those will be really annoying. * Set mouse location on every interaction to fix #87 * Introduce 768-char component name length limit Covers the unlikely edge case that a component name is unreasonably long.
Resolves #47 and also closes #67
This PR fixes the canvas reset issue when the tooltips would overflow the CanvasPage (checkout issue for more context).
The solution to allow overflow on CanvasPage was something of a hunch that made sense and worked. The root cause I think was an edge case of the eventTooltip div overflowing the current div with no room left for the tooltip to reduce to. (Doesn't make much sense describing as text, I discussed this internally with @taneliang to explain better).
Removed empty class
Duration.This PR also truncates long component names with ellipsis.
(Not tested yet, ideally should work. Will mark it ready for review as soon as I can test it with suitable data)