Skip to content

Commit 069edea

Browse files
Nancy LiDevtools-frontend LUCI CQ
authored andcommitted
[Flame chart] Fix the bug for memory panel hovering
The memory panel will not have a group, but just append the entries, So the |HoverType| will always be |OUTSIDE_TRACKS|. But each entry can still be hovered/clicked when users are not hovering any track. So the fix is still do the hovering/clicking behavior for entries even no track is hovered. Fixed: 360795014 Change-Id: Ia0696061d99b5b1e2c90f9e3b237ac6ae430bdf0 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6110498 Reviewed-by: Andres Olivares <[email protected]> Commit-Queue: Nancy Li <[email protected]>
1 parent 18d74db commit 069edea

File tree

1 file changed

+49
-40
lines changed

1 file changed

+49
-40
lines changed

front_end/ui/legacy/components/perf_ui/FlameChart.ts

Lines changed: 49 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -766,8 +766,13 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
766766
this.viewportElement.style.cursor = 'pointer';
767767
return;
768768
case HoverType.INSIDE_TRACK:
769+
case HoverType.OUTSIDE_TRACKS:
769770
this.updateHighlight();
770771
return;
772+
case HoverType.ERROR:
773+
return;
774+
default:
775+
Platform.assertNever(hoverType, `Invalid hovering type: ${hoverType}`);
771776
}
772777
}
773778

@@ -790,7 +795,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
790795
iconTooltip = `Move ${displayName} track down`;
791796
break;
792797
case HoverType.TRACK_CONFIG_HIDE_BUTTON:
793-
if (this.groupIsLastVisibleTopLevel(group)) {
798+
if (this.groupIsLastVisibleTopLevel(groupIndex)) {
794799
iconTooltip = 'Can not hide the last top level track';
795800
} else {
796801
iconTooltip = `Hide ${displayName} track`;
@@ -1024,42 +1029,45 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
10241029

10251030
// If any button is clicked, we should handle the action only and ignore others.
10261031
const {groupIndex, hoverType} = this.coordinatesToGroupIndexAndHoverType(mouseEvent.offsetX, mouseEvent.offsetY);
1027-
if (groupIndex >= 0) {
1028-
switch (hoverType) {
1029-
case HoverType.TRACK_CONFIG_UP_BUTTON:
1030-
this.moveGroupUp(groupIndex);
1031-
return;
1032-
case HoverType.TRACK_CONFIG_DOWN_BUTTON:
1033-
this.moveGroupDown(groupIndex);
1034-
return;
1035-
case HoverType.TRACK_CONFIG_HIDE_BUTTON:
1036-
if (this.groupIsLastVisibleTopLevel(this.rawTimelineData?.groups[groupIndex])) {
1037-
// If this is the last visible top-level group, we will not allow you hiding the track.
1038-
return;
1039-
}
1040-
this.hideGroup(groupIndex);
1041-
return;
1042-
case HoverType.TRACK_CONFIG_SHOW_BUTTON:
1043-
this.showGroup(groupIndex);
1044-
return;
1045-
case HoverType.INSIDE_TRACK_HEADER:
1046-
this.#selectGroup(groupIndex);
1047-
this.toggleGroupExpand(groupIndex);
1048-
return;
1049-
case HoverType.INSIDE_TRACK: {
1050-
this.#selectGroup(groupIndex);
1051-
1052-
const timelineData = this.timelineData();
1053-
if (mouseEvent.shiftKey && this.highlightedEntryIndex !== -1 && timelineData) {
1054-
const start = timelineData.entryStartTimes[this.highlightedEntryIndex];
1055-
const end = start + timelineData.entryTotalTimes[this.highlightedEntryIndex];
1056-
this.chartViewport.setRangeSelection(start, end);
1057-
} else {
1058-
this.chartViewport.onClick(mouseEvent);
1059-
this.dispatchEventToListeners(Events.ENTRY_INVOKED, this.highlightedEntryIndex);
1060-
}
1032+
// There could be a special case, when there is no group and all entries are appended directly, for example the
1033+
// Memory panel.
1034+
// In this case, the |groupIndex| will be -1, and |groups| should be empty.
1035+
// All the functions here can handle the -1 groupIndex properly, so we don't need to add extra check here.
1036+
switch (hoverType) {
1037+
case HoverType.TRACK_CONFIG_UP_BUTTON:
1038+
this.moveGroupUp(groupIndex);
1039+
return;
1040+
case HoverType.TRACK_CONFIG_DOWN_BUTTON:
1041+
this.moveGroupDown(groupIndex);
1042+
return;
1043+
case HoverType.TRACK_CONFIG_HIDE_BUTTON:
1044+
if (this.groupIsLastVisibleTopLevel(groupIndex)) {
1045+
// If this is the last visible top-level group, we will not allow you hiding the track.
10611046
return;
10621047
}
1048+
this.hideGroup(groupIndex);
1049+
return;
1050+
case HoverType.TRACK_CONFIG_SHOW_BUTTON:
1051+
this.showGroup(groupIndex);
1052+
return;
1053+
case HoverType.INSIDE_TRACK_HEADER:
1054+
this.#selectGroup(groupIndex);
1055+
this.toggleGroupExpand(groupIndex);
1056+
return;
1057+
case HoverType.INSIDE_TRACK:
1058+
case HoverType.OUTSIDE_TRACKS: {
1059+
this.#selectGroup(groupIndex);
1060+
1061+
const timelineData = this.timelineData();
1062+
if (mouseEvent.shiftKey && this.highlightedEntryIndex !== -1 && timelineData) {
1063+
const start = timelineData.entryStartTimes[this.highlightedEntryIndex];
1064+
const end = start + timelineData.entryTotalTimes[this.highlightedEntryIndex];
1065+
this.chartViewport.setRangeSelection(start, end);
1066+
} else {
1067+
this.chartViewport.onClick(mouseEvent);
1068+
this.dispatchEventToListeners(Events.ENTRY_INVOKED, this.highlightedEntryIndex);
1069+
}
1070+
return;
10631071
}
10641072
}
10651073
}
@@ -2704,9 +2712,9 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
27042712
// When it is normal mode, there are no icons to the left of a track.
27052713
// When it is in edit mode, there are three icons to customize the groups.
27062714
const iconsWidth = this.#inTrackConfigEditMode ? EDIT_MODE_TOTAL_ICON_WIDTH : 0;
2707-
this.forEachGroupInViewport((offset, index, group) => {
2715+
this.forEachGroupInViewport((offset, groupIndex, group) => {
27082716
context.font = this.#font;
2709-
if (this.isGroupCollapsible(index) && !group.expanded || group.style.shareHeaderLine) {
2717+
if (this.isGroupCollapsible(groupIndex) && !group.expanded || group.style.shareHeaderLine) {
27102718
// In edit mode, we draw an extra rectangle for the save icon.
27112719
const labelBackgroundWidth = this.labelWidthForGroup(context, group);
27122720
const parsedColor = Common.Color.parse(group.style.backgroundColor);
@@ -2752,7 +2760,7 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
27522760
// If this is the last visible top-level group, we will disable the hide action.
27532761
drawIcon(
27542762
context, HIDE_ICON_LEFT, offset, EDIT_ICON_WIDTH, group.hidden ? showIconPath : hideIconPath,
2755-
this.groupIsLastVisibleTopLevel(group) ? '--sys-color-state-disabled' : iconColor);
2763+
this.groupIsLastVisibleTopLevel(groupIndex) ? '--sys-color-state-disabled' : iconColor);
27562764
}
27572765
}
27582766
});
@@ -3724,10 +3732,11 @@ export class FlameChart extends Common.ObjectWrapper.eventMixin<EventTypes, type
37243732
return style.height !== style.itemsHeight;
37253733
}
37263734

3727-
groupIsLastVisibleTopLevel(group?: Group): boolean {
3728-
if (!group) {
3735+
groupIsLastVisibleTopLevel(groupIndex: number): boolean {
3736+
if (groupIndex < 0 || !this.rawTimelineData) {
37293737
return true;
37303738
}
3739+
const group = this.rawTimelineData.groups[groupIndex];
37313740
const visibleTopLevelGroupNumber =
37323741
this.#groupTreeRoot?.children.filter(track => !this.rawTimelineData?.groups[track.index].hidden).length;
37333742
return visibleTopLevelGroupNumber === 1 && group.style.nestingLevel === 0 && !group.hidden;

0 commit comments

Comments
 (0)