Skip to content

Commit da1ad17

Browse files
authored
Track context menus should be accessible from anywhere in the track, fixes #3947. (#5562)
1 parent 41631b5 commit da1ad17

File tree

9 files changed

+128
-82
lines changed

9 files changed

+128
-82
lines changed

src/components/timeline/GlobalTrack.tsx

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ type StateProps = {
6363
readonly globalTrack: GlobalTrack;
6464
readonly isSelected: boolean;
6565
readonly isHidden: boolean;
66-
readonly titleText: string | null;
66+
readonly titleText: string | undefined;
6767
readonly localTrackOrder: TrackIndex[];
6868
readonly localTracks: LocalTrack[];
6969
readonly pid: Pid | null;
@@ -257,20 +257,20 @@ class GlobalTrackComponent extends PureComponent<Props> {
257257
className="timelineTrack"
258258
style={style as React.CSSProperties}
259259
>
260-
<div
261-
className={classNames('timelineTrackRow timelineTrackGlobalRow', {
262-
selected: isSelected,
263-
})}
264-
onClick={this._selectCurrentTrack}
260+
<ContextMenuTrigger
261+
id="TimelineTrackContextMenu"
262+
renderTag="div"
263+
attributes={{
264+
className: classNames('timelineTrackRow timelineTrackGlobalRow', {
265+
selected: isSelected,
266+
}),
267+
onContextMenu: this._onContextMenu,
268+
}}
265269
>
266-
<ContextMenuTrigger
267-
id="TimelineTrackContextMenu"
268-
renderTag="div"
269-
attributes={{
270-
title: titleText,
271-
className: 'timelineTrackLabel timelineTrackGlobalGrippy',
272-
onContextMenu: this._onContextMenu,
273-
}}
270+
<div
271+
className="timelineTrackLabel timelineTrackGlobalGrippy"
272+
title={titleText}
273+
onClick={this._selectCurrentTrack}
274274
>
275275
<button type="button" className="timelineTrackNameButton">
276276
{trackName}
@@ -301,9 +301,14 @@ class GlobalTrackComponent extends PureComponent<Props> {
301301
onClick={this._hideCurrentTrack}
302302
/>
303303
</Localized>
304-
</ContextMenuTrigger>
305-
<div className="timelineTrackTrack">{this.renderTrack()}</div>
306-
</div>
304+
</div>
305+
<div
306+
className="timelineTrackTrack"
307+
onClick={this._selectCurrentTrack}
308+
>
309+
{this.renderTrack()}
310+
</div>
311+
</ContextMenuTrigger>
307312
{localTracks.length > 0 && pid !== null
308313
? this.renderLocalTracks(pid)
309314
: null}
@@ -329,7 +334,7 @@ export const TimelineGlobalTrack = explicitConnect<
329334
// These get assigned based on the track type.
330335
let threadIndex = null;
331336
let isSelected = false;
332-
let titleText = null;
337+
let titleText;
333338

334339
let localTrackOrder = EMPTY_TRACK_ORDER;
335340
let localTracks = EMPTY_LOCAL_TRACKS;

src/components/timeline/LocalTrack.tsx

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ type StateProps = {
5454
readonly trackName: string;
5555
readonly isSelected: boolean;
5656
readonly isHidden: boolean;
57-
readonly titleText: string | null;
57+
readonly titleText: string | undefined;
5858
};
5959

6060
type DispatchProps = {
@@ -151,21 +151,20 @@ class LocalTrackComponent extends PureComponent<Props> {
151151
return (
152152
<li className="timelineTrack timelineTrackLocal" style={style}>
153153
{/* This next div is used to mirror the structure of the TimelineGlobalTrack */}
154-
<div
155-
className={classNames('timelineTrackRow timelineTrackLocalRow', {
156-
selected: isSelected,
157-
})}
158-
onClick={this._selectCurrentTrack}
154+
<ContextMenuTrigger
155+
id="TimelineTrackContextMenu"
156+
renderTag="div"
157+
attributes={{
158+
className: classNames('timelineTrackRow timelineTrackLocalRow', {
159+
selected: isSelected,
160+
}),
161+
onContextMenu: this._onContextMenu,
162+
}}
159163
>
160-
<ContextMenuTrigger
161-
id="TimelineTrackContextMenu"
162-
renderTag="div"
163-
attributes={{
164-
title: titleText,
165-
className:
166-
'timelineTrackLabel timelineTrackLocalLabel timelineTrackLocalGrippy',
167-
onContextMenu: this._onContextMenu,
168-
}}
164+
<div
165+
className="timelineTrackLabel timelineTrackLocalLabel timelineTrackLocalGrippy"
166+
title={titleText}
167+
onClick={this._selectCurrentTrack}
169168
>
170169
<button type="button" className="timelineTrackNameButton">
171170
{trackName}
@@ -178,9 +177,14 @@ class LocalTrackComponent extends PureComponent<Props> {
178177
onClick={this._hideCurrentTrack}
179178
/>
180179
</Localized>
181-
</ContextMenuTrigger>
182-
<div className="timelineTrackTrack">{this.renderTrack()}</div>
183-
</div>
180+
</div>
181+
<div
182+
className="timelineTrackTrack"
183+
onClick={this._selectCurrentTrack}
184+
>
185+
{this.renderTrack()}
186+
</div>
187+
</ContextMenuTrigger>
184188
</li>
185189
);
186190
}
@@ -193,7 +197,7 @@ export const TimelineLocalTrack = explicitConnect<
193197
>({
194198
mapStateToProps: (state, { pid, localTrack, trackIndex }) => {
195199
// These get assigned based on the track type.
196-
let titleText = null;
200+
let titleText;
197201
let isSelected = false;
198202

199203
const selectedThreadIndexes = getSelectedThreadIndexes(state);

src/components/timeline/Markers.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,10 @@ class TimelineMarkers extends React.PureComponent<Props, State> {
470470
isSelected ? 'selected' : null
471471
)}
472472
>
473-
<ContextMenuTrigger id="MarkerContextMenu">
473+
<ContextMenuTrigger
474+
id="MarkerContextMenu"
475+
disable={hoveredMarkerIndex === null && rightClickedMarker === null}
476+
>
474477
<TimelineMarkersCanvas
475478
width={this.props.width}
476479
height={this.props.height}

src/components/timeline/TrackNetwork.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -369,6 +369,9 @@ class Network extends PureComponent<Props, State> {
369369
>
370370
<ContextMenuTrigger
371371
id="MarkerContextMenu"
372+
disable={
373+
hoveredMarkerIndex === null && rightClickedMarkerIndex === null
374+
}
372375
attributes={{
373376
className: 'treeViewContextMenu',
374377
}}

src/test/components/GlobalTrack.test.tsx

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,11 @@ describe('timeline/GlobalTrack', function () {
139139
container.querySelector('.timelineTrackLabel'),
140140
`Couldn't find the track label with selector .timelineTrackLabel`
141141
) as HTMLElement;
142+
const getGlobalTrackContent = () =>
143+
ensureExists(
144+
container.querySelector('.timelineTrackTrack'),
145+
`Couldn't find the track content with selector .timelineTrackTrack`
146+
) as HTMLElement;
142147
const getGlobalTrackRow = () =>
143148
ensureExists(
144149
container.querySelector('.timelineTrackGlobalRow'),
@@ -155,6 +160,7 @@ describe('timeline/GlobalTrack', function () {
155160
trackIndex,
156161
threadIndex,
157162
getGlobalTrackLabel,
163+
getGlobalTrackContent,
158164
getGlobalTrackRow,
159165
};
160166
}
@@ -222,7 +228,7 @@ describe('timeline/GlobalTrack', function () {
222228
expect(getGlobalTrackRow()).toHaveClass('selected');
223229
});
224230

225-
it('can right click a thread', () => {
231+
it('can right click a thread on the label', () => {
226232
const { getState, getGlobalTrackLabel, threadIndex, trackReference } =
227233
setup();
228234

@@ -231,13 +237,22 @@ describe('timeline/GlobalTrack', function () {
231237
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
232238
});
233239

234-
it('can select a thread by clicking the row', () => {
235-
const { getState, getGlobalTrackRow, threadIndex } = setup();
240+
it('can select a thread by clicking the track content', () => {
241+
const { getState, getGlobalTrackContent, threadIndex } = setup();
236242
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
237-
fireFullClick(getGlobalTrackRow());
243+
fireFullClick(getGlobalTrackContent());
238244
expect(getFirstSelectedThreadIndex(getState())).toBe(threadIndex);
239245
});
240246

247+
it('can right click a thread on the track content', () => {
248+
const { getState, getGlobalTrackContent, threadIndex, trackReference } =
249+
setup();
250+
251+
fireFullContextMenu(getGlobalTrackContent());
252+
expect(getRightClickedTrack(getState())).toEqual(trackReference);
253+
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
254+
});
255+
241256
it('will render a stub div if the track is hidden', () => {
242257
const { container, trackIndex, dispatch } = setup();
243258
act(() => {

src/test/components/LocalTrack.test.tsx

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ describe('timeline/LocalTrack', function () {
7878
expect(getLocalTrackRow()).toHaveClass('selected');
7979
});
8080

81-
it('can right click a thread', () => {
81+
it('can right click a thread on the label', () => {
8282
const { getState, getLocalTrackLabel, threadIndex, trackReference } =
8383
setupThreadTrack();
8484

@@ -87,13 +87,23 @@ describe('timeline/LocalTrack', function () {
8787
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
8888
});
8989

90-
it('can select a thread by clicking the row', () => {
91-
const { getState, getLocalTrackRow, threadIndex } = setupThreadTrack();
90+
it('can select a thread by clicking the track content', () => {
91+
const { getState, getLocalTrackContent, threadIndex } =
92+
setupThreadTrack();
9293
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
93-
fireFullClick(getLocalTrackRow());
94+
fireFullClick(getLocalTrackContent());
9495
expect(getFirstSelectedThreadIndex(getState())).toBe(threadIndex);
9596
});
9697

98+
it('can right click a thread on the track content', () => {
99+
const { getState, getLocalTrackContent, threadIndex, trackReference } =
100+
setupThreadTrack();
101+
102+
fireFullContextMenu(getLocalTrackContent());
103+
expect(getRightClickedTrack(getState())).toEqual(trackReference);
104+
expect(getFirstSelectedThreadIndex(getState())).not.toBe(threadIndex);
105+
});
106+
97107
it('will render a stub div if the track is hidden', () => {
98108
const { container, pid, trackReference, dispatch } = setupThreadTrack();
99109
act(() => {
@@ -203,6 +213,11 @@ function setup(
203213
container.querySelector('.timelineTrackLabel'),
204214
`Couldn't find the track label with selector .timelineTrackLabel`
205215
) as HTMLElement;
216+
const getLocalTrackContent = () =>
217+
ensureExists(
218+
container.querySelector('.timelineTrackTrack'),
219+
`Couldn't find the track content with selector .timelineTrackTrack`
220+
) as HTMLElement;
206221
const getLocalTrackRow = () =>
207222
ensureExists(
208223
container.querySelector('.timelineTrackLocalRow'),
@@ -219,6 +234,7 @@ function setup(
219234
threadIndex,
220235
pid: PID,
221236
getLocalTrackLabel,
237+
getLocalTrackContent,
222238
getLocalTrackRow,
223239
flushRafCalls,
224240
};

0 commit comments

Comments
 (0)