Skip to content

Clean up hover test. #9826

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kenzieschmoll wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Clean up hover test. #9826

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
09a2c25
Clean up hover test.
kenzieschmoll May 8, 2026
e1d7bee
formatting
kenzieschmoll May 8, 2026
51fa4a6
Merge branch 'tooltipfollowup' of github.com:kenzieschmoll/devtools; …
kenzieschmoll May 8, 2026
1c0cf2f
Merge branch 'master' of github.com:flutter/devtools into tooltipfoll…
kenzieschmoll May 8, 2026
39b9276
pubspec lock
kenzieschmoll May 8, 2026
d025901
formatting
kenzieschmoll May 8, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 17 additions & 18 deletions packages/devtools_app/lib/src/shared/ui/hover.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ int _hoverIndexFor(double dx, TextSpan line) {
const _hoverYOffset = 10.0;

/// Minimum distance from the side of screen to show tooltip
const _hoverMargin = 16.0;
@visibleForTesting
const hoverMargin = 16.0;

/// Defines how a [HoverCardTooltip] is positioned
enum HoverCardPosition {
Expand Down Expand Up @@ -256,19 +257,16 @@ class HoverCard {
final overlaySize = overlayBox.size;
final localPosition = overlayBox.globalToLocal(event.position);

final maxX = math.max(
_hoverMargin,
overlaySize.width - _hoverMargin - width,
);
final x = (localPosition.dx - (width / 2.0)).clamp(_hoverMargin, maxX);
final maxX = math.max(hoverMargin, overlaySize.width - hoverMargin - width);
final x = (localPosition.dx - (width / 2.0)).clamp(hoverMargin, maxX);

final maxY = math.max(
_hoverMargin,
hoverMargin,
overlaySize.height -
_hoverMargin -
hoverMargin -
_totalMaxHoverCardHeight(hasTitle: title != null),
);
final y = (localPosition.dy + _hoverYOffset).clamp(_hoverMargin, maxY);
final y = (localPosition.dy + _hoverYOffset).clamp(hoverMargin, maxY);

return Offset(x, y);
}
Expand Down Expand Up @@ -381,7 +379,8 @@ class HoverCardTooltip extends StatefulWidget {
}) : asyncGenerateHoverCardData = null,
asyncTimeout = null;

static const _hoverDelay = Duration(milliseconds: 500);
@visibleForTesting
static const hoverDelay = Duration(milliseconds: 500);
static const defaultHoverWidth = 450.0;

/// Whether the tooltip is currently enabled.
Expand Down Expand Up @@ -420,7 +419,7 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {

void _onHoverExit() {
_showTimer?.cancel();
_removeTimer = Timer(HoverCardTooltip._hoverDelay, () {
_removeTimer = Timer(HoverCardTooltip.hoverDelay, () {
if (_currentHoverCard != null) {
_hoverCardController.maybeRemoveHoverCard(_currentHoverCard!);
}
Expand Down Expand Up @@ -448,7 +447,7 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
final generateHoverCardData = widget.generateHoverCardData;
final asyncTimeout = widget.asyncTimeout;

_showTimer = Timer(HoverCardTooltip._hoverDelay, () {
_showTimer = Timer(HoverCardTooltip.hoverDelay, () {
if (asyncGenerateHoverCardData != null) {
assert(generateHoverCardData == null);
_showAsyncHoverCard(
Expand Down Expand Up @@ -596,13 +595,13 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
final box = context.findRenderObject() as RenderBox;

final maxX = math.max(
_hoverMargin,
overlayBox.size.width - _hoverMargin - width,
hoverMargin,
overlayBox.size.width - hoverMargin - width,
);
final maxY = math.max(
_hoverMargin,
hoverMargin,
overlayBox.size.height -
_hoverMargin -
hoverMargin -
_totalMaxHoverCardHeight(hasTitle: title != null),
);

Expand All @@ -612,8 +611,8 @@ class _HoverCardTooltipState extends State<HoverCardTooltip> {
);

return Offset(
offset.dx.clamp(_hoverMargin, maxX),
offset.dy.clamp(_hoverMargin, maxY),
offset.dx.clamp(hoverMargin, maxX),
offset.dy.clamp(hoverMargin, maxY),
);
}

Expand Down
118 changes: 59 additions & 59 deletions packages/devtools_app/test/shared/ui/hover_positioning_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ import 'package:devtools_test/helpers.dart';
import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
import 'package:provider/provider.dart';

const _windowWidth = 800.0;
const _windowHeight = 600.0;
const _windowSize = Size(_windowWidth, _windowHeight);

void main() {
Future<void> pumpHoverCardTooltip(
Expand Down Expand Up @@ -40,13 +43,13 @@ void main() {
await gesture.addPointer(location: Offset.zero);
final center = tester.getCenter(find.text('Hover Me'));
await gesture.moveTo(center);
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(HoverCardTooltip.hoverDelay);
await tester.pumpAndSettle();
}

testWidgetsWithWindowSize(
'HoverCard at the bottom of the window should not overflow',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
// Use a title to increase the height beyond the base content height.
await pumpHoverCardTooltip(
Expand All @@ -66,14 +69,16 @@ void main() {
final position = renderBox.localToGlobal(Offset.zero);
final size = renderBox.size;

// _hoverMargin = 16.0
expect(position.dy + size.height, lessThanOrEqualTo(600.0 - 16.0));
expect(
position.dy + size.height,
lessThanOrEqualTo(_windowHeight - hoverMargin),
);
},
);

testWidgetsWithWindowSize(
'HoverCard at the right of the window should not overflow',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
await pumpHoverCardTooltip(tester, alignment: Alignment.centerRight);

Expand All @@ -88,8 +93,10 @@ void main() {
final position = renderBox.localToGlobal(Offset.zero);
final size = renderBox.size;

// _hoverMargin = 16.0
expect(position.dx + size.width, lessThanOrEqualTo(800.0 - 16.0));
expect(
position.dx + size.width,
lessThanOrEqualTo(_windowWidth - hoverMargin),
);
},
);

Expand All @@ -110,38 +117,36 @@ void main() {
},
);

testWidgetsWithWindowSize(
'HoverCard height clamping with title',
const Size(800, 600),
(WidgetTester tester) async {
await pumpHoverCardTooltip(
tester,
alignment: Alignment.bottomCenter,
title: 'An Important Title',
);
testWidgetsWithWindowSize('HoverCard height clamping with title', _windowSize, (
WidgetTester tester,
) async {
await pumpHoverCardTooltip(
tester,
alignment: Alignment.bottomCenter,
title: 'An Important Title',
);

final hoverContentFinderWithTitle = find.text('Hover Content');
expect(hoverContentFinderWithTitle, findsOneWidget);
final hoverContentFinderWithTitle = find.text('Hover Content');
expect(hoverContentFinderWithTitle, findsOneWidget);

final containerWithTitle = find
.ancestor(
of: hoverContentFinderWithTitle,
matching: find.byType(Container),
)
.last;
final containerWithTitle = find
.ancestor(
of: hoverContentFinderWithTitle,
matching: find.byType(Container),
)
.last;

final renderBoxWithTitle =
tester.renderObject(containerWithTitle) as RenderBox;
final positionWithTitle = renderBoxWithTitle.localToGlobal(Offset.zero);
final renderBoxWithTitle =
tester.renderObject(containerWithTitle) as RenderBox;
final positionWithTitle = renderBoxWithTitle.localToGlobal(Offset.zero);

// Clamps strictly at y = 274.0 because of dynamic height containing title/divider.
expect(positionWithTitle.dy, equals(274.0));
},
);
// Clamps strictly at y = 274.0 because of dynamic height containing title/divider.
expect(positionWithTitle.dy, equals(274.0));
Comment on lines +143 to +144
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

[CONCERN] This test uses a hardcoded magic number 274.0 for the expected position. Since the PR aims to clean up the test and already introduces constants like _windowHeight and makes hoverMargin public, this expectation should be calculated relative to those constants. This makes the test more robust against changes to the window size or margin values. Additionally, using the actual height of the rendered box ensures the test correctly verifies that the bottom of the card is positioned at the expected margin from the window edge.

    expect(
      positionWithTitle.dy + renderBoxWithTitle.size.height,
      equals(_windowHeight - hoverMargin),
    );
References
  1. Strict avoidance of raw values in UI (use named constants) and general code quality (DRY, Meaningful Naming). (link)

});

testWidgetsWithWindowSize(
'HoverCard height clamping without title',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
await pumpHoverCardTooltip(tester, alignment: Alignment.bottomCenter);

Expand All @@ -166,39 +171,34 @@ void main() {

testWidgetsWithWindowSize(
'HoverCard translates global coordinates to local coordinates for offset overlays',
const Size(800, 600),
_windowSize,
(WidgetTester tester) async {
final overlayKey = GlobalKey();

await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Padding(
padding: const EdgeInsets.only(left: 50.0, top: 100.0),
child: Provider<HoverCardController>.value(
value: HoverCardController(),
child: Overlay(
key: overlayKey,
initialEntries: [
OverlayEntry(
builder: (context) => Align(
alignment: Alignment.topLeft,
child: HoverCardTooltip.sync(
enabled: () => true,
generateHoverCardData: (event) => HoverCardData(
contents: const SizedBox(
width: 200,
height: 250,
child: Text('Hover Content'),
),
),
child: const Text('Hover Me Offset'),
wrapSimple(
Padding(
padding: const EdgeInsets.only(left: 50.0, top: 100.0),
child: Overlay(
key: overlayKey,
initialEntries: [
OverlayEntry(
builder: (context) => Align(
alignment: Alignment.topLeft,
child: HoverCardTooltip.sync(
enabled: () => true,
generateHoverCardData: (event) => HoverCardData(
contents: const SizedBox(
width: 200,
height: 250,
child: Text('Hover Content'),
),
),
child: const Text('Hover Me Offset'),
),
],
),
),
),
],
),
),
),
Expand All @@ -210,7 +210,7 @@ void main() {

final center = tester.getCenter(find.text('Hover Me Offset'));
await gesture.moveTo(center);
await tester.pump(const Duration(milliseconds: 500));
await tester.pump(HoverCardTooltip.hoverDelay);
await tester.pumpAndSettle();

final hoverContentFinder = find.text('Hover Content');
Expand Down
4 changes: 2 additions & 2 deletions pubspec.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -951,10 +951,10 @@ packages:
dependency: transitive
description:
name: url_launcher_web
sha256: d0412fcf4c6b31ecfdb7762359b7206ffba3bbffd396c6d9f9c4616ece476c1f
sha256: "85c81589622fbc87c1c683aaea164d3604a7777495a79d91e39ffcdec39ddb34"
url: "https://pub.dev"
source: hosted
version: "2.4.2"
version: "2.4.3"
url_launcher_windows:
dependency: transitive
description:
Expand Down
Loading