Hover fixes #3065
Hover fixes #3065
Conversation
| @@ -632,7 +632,7 @@ class LineItem extends StatefulWidget { | |||
| this.activeSearchMatch, | |||
| }) : super(key: key); | |||
|
|
|||
| static const _hoverDelay = Duration(milliseconds: 50); | |||
| static const _hoverDelay = Duration(milliseconds: 150); | |||
There was a problem hiding this comment.
We don't want to show the hover card too quickly. This feels about right.
| @@ -684,6 +687,8 @@ class _LineItemState extends State<LineItem> { | |||
| if (response is! InstanceRef) return; | |||
| final variable = Variable.fromRef(response); | |||
| await _debuggerController.buildVariablesTree(variable); | |||
| if (_hasMouseExited) return; | |||
There was a problem hiding this comment.
we should do more. _onHoverExit only gets called if you changed the line. If the user moved more than a couple pixels within the same line, this hover is also obsolete.
we need to verify that we haven't gotten a mouseMove to a different location within the line as opposed to an exit from the line.
There was a problem hiding this comment.
We already handle that scenario. If the hoverWord is the same we don't change the hover card just like in Chrome.
|
|
||
| void _onHoverExit() { | ||
| _showTimer?.cancel(); | ||
| _hasMouseExited = true; | ||
| _removeTimer = Timer(LineItem._hoverDelay, () { |
There was a problem hiding this comment.
this should be a separate delay. I don't think this delay should be made longer.
There was a problem hiding this comment.
I disagree. We are less likely to "flash" a hover card if a user accidentally moves outside the line.
There was a problem hiding this comment.
I agree with jacob. Look at the difference between the hover card show and hover card hide delays in IntelliJ. There is a longer delay to show than exit.
There was a problem hiding this comment.
I can shorten the delay but we don't want it too small as we want to allow users to hover "into" the card which keeps it around. Otherwise it's frustrating to expand variables displayed in the hover card.
There was a problem hiding this comment.
i would hope this code path isn't needed to make the case of hovering into a card work. ideally the area treated as in the card and the current location should have a border so you don't need to rely on the delay to move over the hover crd.
There was a problem hiding this comment.
That is not the case. Note that the card is rendered below the source line which is why this is required. This also mirrors the Chrome DevTools behavior. There appears to be a grace period where you can move your mouse outside the hovercard and into before it is removed.
|
PTAL |
| child: Container( | ||
| constraints: const BoxConstraints(maxHeight: 250.0), |
There was a problem hiding this comment.
make 250.0 a const var on the theme. maxHoverCardHeight or similar
Closes #3062