Skip to content

Commit 9780f65

Browse files
eamodioeamodio
committed
Resets Graph tracking counters on visibility change
1 parent 6bdab5a commit 9780f65

File tree

8 files changed

+119
-72
lines changed

8 files changed

+119
-72
lines changed

‎src/system/counter.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
const maxSmallIntegerV8 = 2 ** 30 - 1; // Max number that can be stored in V8's smis (small integers)
22

3-
export type Counter = { readonly current: number; next(): number };
3+
export type Counter = { readonly current: number; next(): number; reset(): void };
44

55
export function getScopedCounter(): Counter {
66
let counter = 0;
@@ -14,5 +14,8 @@ export function getScopedCounter(): Counter {
1414
}
1515
return ++counter;
1616
},
17+
reset: function () {
18+
counter = 0;
19+
},
1720
};
1821
}

‎src/webviews/apps/plus/graph/graph-app.ts‎

Lines changed: 72 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ import './sidebar/sidebar';
2828

2929
@customElement('gl-graph-app')
3030
export class GraphApp extends SignalWatcher(LitElement) {
31+
private _hoverTrackingCounter = getScopedCounter();
32+
private _selectionTrackingCounter = getScopedCounter();
33+
3134
// use Light DOM
3235
protected override createRenderRoot(): HTMLElement | DocumentFragment {
3336
return this;
@@ -54,6 +57,63 @@ export class GraphApp extends SignalWatcher(LitElement) {
5457
@query('gl-graph-minimap-container')
5558
minimapEl: GlGraphMinimapContainer | undefined;
5659

60+
onWebviewVisibilityChanged(visible: boolean): void {
61+
if (!visible) return;
62+
63+
this._hoverTrackingCounter.reset();
64+
this._selectionTrackingCounter.reset();
65+
}
66+
67+
resetHover() {
68+
this.graphHover.reset();
69+
}
70+
71+
override render() {
72+
return html`
73+
<div class="graph">
74+
<gl-graph-header
75+
class="graph__header"
76+
@gl-select-commits=${this.handleHeaderSearchNavigation}
77+
></gl-graph-header>
78+
<div class="graph__workspace">
79+
${when(!this.state.allowed, () => html`<gl-graph-gate class="graph__gate"></gl-graph-gate>`)}
80+
<main id="main" class="graph__panes">
81+
<div class="graph__graph-pane">
82+
${when(
83+
this.state.config?.minimap !== false,
84+
() => html`
85+
<gl-graph-minimap-container
86+
.activeDay=${this.graphApp.activeDay}
87+
.disabled=${!this.state.config?.minimap}
88+
.rows=${this.state.rows ?? []}
89+
.rowsStats=${this.state.rowsStats}
90+
.dataType=${this.state.config?.minimapDataType ?? 'commits'}
91+
.markerTypes=${this.state.config?.minimapMarkerTypes ?? []}
92+
.refMetadata=${this.state.refsMetadata}
93+
.searchResults=${this.graphApp.searchResults}
94+
.visibleDays=${this.graphApp.visibleDays}
95+
@gl-graph-minimap-selected=${this.handleMinimapDaySelected}
96+
></gl-graph-minimap-container>
97+
`,
98+
)}
99+
${when(this.state.config?.sidebar, () => html`<gl-graph-sidebar></gl-graph-sidebar>`)}
100+
<gl-graph-hover id="commit-hover" distance=${0} skidding=${15}></gl-graph-hover>
101+
<gl-graph-wrapper
102+
@gl-graph-change-selection=${this.handleGraphSelectionChanged}
103+
@gl-graph-change-visible-days=${this.handleGraphVisibleDaysChanged}
104+
@gl-graph-mouse-leave=${this.handleGraphMouseLeave}
105+
@gl-graph-row-context-menu=${this.handleGraphRowContextMenu}
106+
@gl-graph-row-hover=${this.handleGraphRowHover}
107+
@gl-graph-row-unhover=${this.handleGraphRowUnhover}
108+
></gl-graph-wrapper>
109+
</div>
110+
<!-- future: commit details -->
111+
</main>
112+
</div>
113+
</div>
114+
`;
115+
}
116+
57117
private handleHeaderSearchNavigation(e: CustomEventType<'gl-select-commits'>) {
58118
this.graph.selectCommits([e.detail], false, true);
59119
}
@@ -86,8 +146,18 @@ export class GraphApp extends SignalWatcher(LitElement) {
86146
}
87147
}
88148

89-
private handleGraphSelectionChanged(_e: CustomEventType<'gl-graph-change-selection'>) {
149+
private handleGraphSelectionChanged(e: CustomEventType<'gl-graph-change-selection'>) {
90150
this.graphHover.hide();
151+
152+
const count = this._selectionTrackingCounter.next();
153+
if (count === 1 || count % 100 === 0) {
154+
queueMicrotask(() =>
155+
this._telemetry.sendEvent({
156+
name: 'graph/row/selected',
157+
data: { rows: e.detail.selection.length, count: count },
158+
}),
159+
);
160+
}
91161
}
92162

93163
private handleGraphVisibleDaysChanged({ detail }: CustomEventType<'gl-graph-change-visible-days'>) {
@@ -140,16 +210,14 @@ export class GraphApp extends SignalWatcher(LitElement) {
140210
this.graphHover.onRowUnhovered(graphRow, relatedTarget);
141211
}
142212

143-
private _hoverCounter = getScopedCounter();
144-
145213
private async getRowHoverPromise(row: GraphRow) {
146214
try {
147215
const request = await this._ipc.sendRequest(GetRowHoverRequest, {
148216
type: row.type as GitGraphRowType,
149217
id: row.sha,
150218
});
151219

152-
const count = this._hoverCounter.next();
220+
const count = this._hoverTrackingCounter.next();
153221
if (count === 1 || count % 100 === 0) {
154222
queueMicrotask(() => this._telemetry.sendEvent({ name: 'graph/row/hovered', data: { count: count } }));
155223
}
@@ -163,54 +231,4 @@ export class GraphApp extends SignalWatcher(LitElement) {
163231
private handleGraphMouseLeave() {
164232
this.minimapEl?.unselect(undefined, true);
165233
}
166-
167-
resetHover() {
168-
this.graphHover.reset();
169-
}
170-
171-
override render() {
172-
return html`
173-
<div class="graph">
174-
<gl-graph-header
175-
class="graph__header"
176-
@gl-select-commits=${this.handleHeaderSearchNavigation}
177-
></gl-graph-header>
178-
<div class="graph__workspace">
179-
${when(!this.state.allowed, () => html`<gl-graph-gate class="graph__gate"></gl-graph-gate>`)}
180-
<main id="main" class="graph__panes">
181-
<div class="graph__graph-pane">
182-
${when(
183-
this.state.config?.minimap !== false,
184-
() => html`
185-
<gl-graph-minimap-container
186-
.activeDay=${this.graphApp.activeDay}
187-
.disabled=${!this.state.config?.minimap}
188-
.rows=${this.state.rows ?? []}
189-
.rowsStats=${this.state.rowsStats}
190-
.dataType=${this.state.config?.minimapDataType ?? 'commits'}
191-
.markerTypes=${this.state.config?.minimapMarkerTypes ?? []}
192-
.refMetadata=${this.state.refsMetadata}
193-
.searchResults=${this.graphApp.searchResults}
194-
.visibleDays=${this.graphApp.visibleDays}
195-
@gl-graph-minimap-selected=${this.handleMinimapDaySelected}
196-
></gl-graph-minimap-container>
197-
`,
198-
)}
199-
${when(this.state.config?.sidebar, () => html`<gl-graph-sidebar></gl-graph-sidebar>`)}
200-
<gl-graph-hover id="commit-hover" distance=${0} skidding=${15}></gl-graph-hover>
201-
<gl-graph-wrapper
202-
@gl-graph-change-selection=${this.handleGraphSelectionChanged}
203-
@gl-graph-change-visible-days=${this.handleGraphVisibleDaysChanged}
204-
@gl-graph-mouse-leave=${this.handleGraphMouseLeave}
205-
@gl-graph-row-context-menu=${this.handleGraphRowContextMenu}
206-
@gl-graph-row-hover=${this.handleGraphRowHover}
207-
@gl-graph-row-unhover=${this.handleGraphRowUnhover}
208-
></gl-graph-wrapper>
209-
</div>
210-
<!-- future: commit details -->
211-
</main>
212-
</div>
213-
</div>
214-
`;
215-
}
216234
}

‎src/webviews/apps/plus/graph/graph-wrapper/graph-wrapper.ts‎

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import { customElement, query } from 'lit/decorators.js';
77
import { ifDefined } from 'lit/directives/if-defined.js';
88
import type { GitGraphRowType } from '../../../../../git/models/graph';
99
import { filterMap } from '../../../../../system/array';
10-
import { getScopedCounter } from '../../../../../system/counter';
1110
import {
1211
DoubleClickedCommandType,
1312
GetMissingAvatarsCommand,
@@ -177,8 +176,6 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) {
177176
this.dispatchEvent(new CustomEvent('gl-graph-row-unhover', { detail: detail }));
178177
}
179178

180-
private _selectionCounter = getScopedCounter();
181-
182179
private onSelectionChanged({ detail: rows }: CustomEventType<'graph-changeselection'>) {
183180
const selection = filterMap(rows, r =>
184181
r != null ? { id: r.sha, type: r.type as GitGraphRowType } : undefined,
@@ -191,16 +188,6 @@ export class GlGraphWrapper extends SignalWatcher(LitElement) {
191188

192189
this.dispatchEvent(new CustomEvent('gl-graph-change-selection', { detail: { selection: selection } }));
193190
this._ipc.sendCommand(UpdateSelectionCommand, { selection: selection });
194-
195-
const count = this._selectionCounter.next();
196-
if (count === 1 || count % 100 === 0) {
197-
queueMicrotask(() =>
198-
this._telemetry.sendEvent({
199-
name: 'graph/row/selected',
200-
data: { rows: selection.length, count: count },
201-
}),
202-
);
203-
}
204191
}
205192

206193
private onVisibleDaysChanged({ detail }: CustomEventType<'graph-changevisibledays'>) {

‎src/webviews/apps/plus/graph/graph.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,10 @@ export class GraphAppHost extends GlAppHost<State> {
170170
this.applyTheme(th);
171171
}
172172

173+
protected override onWebviewVisibilityChanged(visible: boolean): void {
174+
this.appElement?.onWebviewVisibilityChanged(visible);
175+
}
176+
173177
private applyTheme(theme: { cssVariables: CssVariables; themeOpacityFactor: number }) {
174178
this._graphState.theming = theme;
175179
}

‎src/webviews/apps/shared/appBase.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,12 @@ import type {
1111
IpcRequest,
1212
WebviewFocusChangedParams,
1313
} from '../../protocol';
14-
import { DidChangeWebviewFocusNotification, WebviewFocusChangedCommand, WebviewReadyCommand } from '../../protocol';
14+
import {
15+
DidChangeWebviewFocusNotification,
16+
DidChangeWebviewVisibilityNotification,
17+
WebviewFocusChangedCommand,
18+
WebviewReadyCommand,
19+
} from '../../protocol';
1520
import { ipcContext } from './contexts/ipc';
1621
import { loggerContext, LoggerContext } from './contexts/logger';
1722
import { PromosContext, promosContext } from './contexts/promos';
@@ -108,6 +113,12 @@ export abstract class App<
108113
);
109114
break;
110115

116+
case DidChangeWebviewVisibilityNotification.is(msg):
117+
window.dispatchEvent(
118+
new CustomEvent(msg.params.visible ? 'webview-visible' : 'webview-hidden'),
119+
);
120+
break;
121+
111122
default:
112123
this.onMessageReceived!(msg);
113124
}

‎src/webviews/apps/shared/appHost.ts‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,12 @@ import type { CustomEditorIds, WebviewIds, WebviewViewIds } from '../../../const
66
import type { Deferrable } from '../../../system/function/debounce';
77
import { debounce } from '../../../system/function/debounce';
88
import type { WebviewFocusChangedParams } from '../../protocol';
9-
import { DidChangeWebviewFocusNotification, WebviewFocusChangedCommand, WebviewReadyCommand } from '../../protocol';
9+
import {
10+
DidChangeWebviewFocusNotification,
11+
DidChangeWebviewVisibilityNotification,
12+
WebviewFocusChangedCommand,
13+
WebviewReadyCommand,
14+
} from '../../protocol';
1015
import { GlElement } from './components/element';
1116
import { ipcContext } from './contexts/ipc';
1217
import { loggerContext, LoggerContext } from './contexts/logger';
@@ -64,7 +69,9 @@ export abstract class GlAppHost<
6469
private _stateProvider!: StateProvider<State>;
6570

6671
protected abstract createStateProvider(state: State, ipc: HostIpc): StateProvider<State>;
67-
protected onPersistState(_state: State): void {}
72+
protected onPersistState?(state: State): void;
73+
protected onWebviewFocusChanged?(focused: boolean): void;
74+
protected onWebviewVisibilityChanged?(visible: boolean): void;
6875

6976
override connectedCallback(): void {
7077
super.connectedCallback();
@@ -78,7 +85,7 @@ export abstract class GlAppHost<
7885
const state = this.bootstrap;
7986
this.bootstrap = undefined!;
8087
this._ipc.replaceIpcPromisesWithPromises(state);
81-
this.onPersistState(state);
88+
this.onPersistState?.(state);
8289

8390
const themeEvent = computeThemeColors();
8491
if (this.onThemeUpdated != null) {
@@ -92,8 +99,15 @@ export abstract class GlAppHost<
9299
this._ipc.onReceiveMessage(msg => {
93100
switch (true) {
94101
case DidChangeWebviewFocusNotification.is(msg):
102+
this.onWebviewFocusChanged?.(msg.params.focused);
95103
window.dispatchEvent(new CustomEvent(msg.params.focused ? 'webview-focus' : 'webview-blur'));
96104
break;
105+
case DidChangeWebviewVisibilityNotification.is(msg):
106+
this.onWebviewVisibilityChanged?.(msg.params.visible);
107+
window.dispatchEvent(
108+
new CustomEvent(msg.params.visible ? 'webview-visible' : 'webview-hidden'),
109+
);
110+
break;
97111
}
98112
}),
99113
this._ipc,

‎src/webviews/protocol.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,14 @@ export const DidChangeWebviewFocusNotification = new IpcCommand<DidChangeWebview
156156
'webview/focus/didChange',
157157
);
158158

159+
export interface DidChangeWebviewVisibilityParams {
160+
visible: boolean;
161+
}
162+
export const DidChangeWebviewVisibilityNotification = new IpcNotification<DidChangeWebviewVisibilityParams>(
163+
'core',
164+
'webview/visibility/didChange',
165+
);
166+
159167
export interface DidChangeConfigurationParams {
160168
config: Config;
161169
customSettings: Record<string, boolean>;

‎src/webviews/webviewController.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import {
3131
ApplicablePromoRequest,
3232
DidChangeHostWindowFocusNotification,
3333
DidChangeWebviewFocusNotification,
34+
DidChangeWebviewVisibilityNotification,
3435
ExecuteCommand,
3536
ipcPromiseSettled,
3637
isIpcPromise,
@@ -555,6 +556,7 @@ export class WebviewController<
555556
this.handleFocusChanged(false);
556557
}
557558

559+
void this.notify(DidChangeWebviewVisibilityNotification, { visible: visible });
558560
this.provider.onVisibilityChanged?.(visible);
559561
}
560562

0 commit comments

Comments
 (0)