Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions locales/en-US/app.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -438,6 +438,8 @@ MarkerContextMenu--end-selection-at-marker-start =
End selection at marker’s <strong>start</strong>
MarkerContextMenu--end-selection-at-marker-end =
End selection at marker’s <strong>end</strong>
MarkerContextMenu--override-zero-at-marker-start =
Override zero at marker’s start
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this concept is going to be clear to localizers. Is "zero" just the start?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. It comes from "getZeroAt" name, but it's not obvious.

Other options I can think of are the following:

  • Override the start of the timeline with marker's start
  • Set the start of the timeline with marker's start
  • Override the timeline origin with marker's start
  • Set the timeline origin with marker's start
  • Start the timeline from marker's start

And the following for the menu button:

  • Starts at 100ms
  • Starts from 100ms
  • Origin at 100ms
  • Origin overridden to 100ms

Can I have your opinion?
If there are better labels, let me know.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's wait for @canova and folks who understand the project — and target audience — better than me.

Personally, I would go with something like:

  • Align the timeline start with the marker start(or Set… to).
  • Starting point moved to 100ms

MarkerContextMenu--copy-description = Copy description
MarkerContextMenu--copy-call-stack = Copy call stack
MarkerContextMenu--copy-url = Copy URL
Expand Down Expand Up @@ -681,6 +683,12 @@ MenuButtons--publish--download = Download
MenuButtons--publish--compressing = Compressing…
MenuButtons--publish--error-while-compressing = Error while compressing, try unchecking some checkboxes to reduce the profile size.

# This string is the button's label, where the button is shown when the "zero"
# point of the timeline is overridden.
# Variables:
# $zeroAt (String) - The timestamp of the overridden "zero"
MenuButtons--zero-at = Zero at <span>{ $zeroAt }</span>

## NetworkSettings
## This is used in the network chart.

Expand Down
7 changes: 7 additions & 0 deletions src/actions/profile-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1905,6 +1905,13 @@ export function changeMouseTimePosition(
};
}

export function overrideZeroAt(zeroAt: Milliseconds | null): Action {
return {
type: 'OVERRIDE_ZERO_AT',
zeroAt,
};
}

export function changeTableViewOptions(
tab: TabSlug,
tableViewOptions: TableViewOptions
Expand Down
9 changes: 9 additions & 0 deletions src/components/app/MenuButtons/index.css
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,15 @@
background-image: url(../../../../res/img/svg/maximize-dark-12.svg);
}

.menuButtonsResetZeroAtButton::before {
background-image: url(../../../../res/img/svg/undo-dark-12.svg);
}

.menuButtonsZeroAtTimestamp {
font-weight: bold;
margin-inline-start: 0.3em;
}

.profileInfoUploadedActions {
padding: 8px 0 8px 40px; /* The 40px padding leaves the room for the cloud image */
border-bottom: 1px solid rgb(0 0 0 / 0.05);
Expand Down
44 changes: 43 additions & 1 deletion src/components/app/MenuButtons/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ import classNames from 'classnames';
import { Localized } from '@fluent/react';

import explicitConnect from 'firefox-profiler/utils/connect';
import { getProfileRootRange } from 'firefox-profiler/selectors/profile';
import {
getProfileRootRange,
getOverriddenZeroAtTimestamp,
} from 'firefox-profiler/selectors/profile';
import {
getDataSource,
getProfileUrl,
Expand All @@ -36,6 +39,7 @@ import {
dismissNewlyPublished,
profileRemotelyDeleted,
} from 'firefox-profiler/actions/app';
import { overrideZeroAt } from 'firefox-profiler/actions/profile-view';

import {
getAbortFunction,
Expand Down Expand Up @@ -72,12 +76,14 @@ type StateProps = {
readonly hasPrePublishedState: boolean;
readonly abortFunction: () => void;
readonly currentProfileUploadedInformation: UploadedProfileInformation | null;
readonly getOverriddenZeroAtTimestamp: string | null;
};

type DispatchProps = {
readonly dismissNewlyPublished: typeof dismissNewlyPublished;
readonly revertToPrePublishedState: typeof revertToPrePublishedState;
readonly profileRemotelyDeleted: typeof profileRemotelyDeleted;
readonly overrideZeroAt: typeof overrideZeroAt;
};

type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
Expand Down Expand Up @@ -130,6 +136,11 @@ class MenuButtonsImpl extends React.PureComponent<Props, State> {
});
};

_showZeroAtMenu = () => {
const { overrideZeroAt } = this.props;
overrideZeroAt(null);
};

_renderUploadedProfileActions(
currentProfileUploadedInformation: UploadedProfileInformation
) {
Expand Down Expand Up @@ -309,6 +320,34 @@ class MenuButtonsImpl extends React.PureComponent<Props, State> {
) : null;
}

_renderZeroAt() {
const { getOverriddenZeroAtTimestamp } = this.props;
if (getOverriddenZeroAtTimestamp === null) {
return null;
}

return (
<button
type="button"
className="menuButtonsButton menuButtonsButton-hasIcon menuButtonsResetZeroAtButton"
onClick={this._showZeroAtMenu}
>
<Localized
id="MenuButtons--zero-at"
attrs={{ title: true }}
vars={{ zeroAt: getOverriddenZeroAtTimestamp }}
elems={{
span: <span className="menuButtonsZeroAtTimestamp" />,
}}
>
<>
Zero at <span>{getOverriddenZeroAtTimestamp}</span>
</>
</Localized>
</button>
);
}

_renderRevertProfile() {
const { hasPrePublishedState, revertToPrePublishedState } = this.props;
if (!hasPrePublishedState) {
Expand All @@ -330,6 +369,7 @@ class MenuButtonsImpl extends React.PureComponent<Props, State> {
override render() {
return (
<>
{this._renderZeroAt()}
{this._renderRevertProfile()}
{this._renderMetaInfoButton()}
{this._renderPublishPanel()}
Expand Down Expand Up @@ -360,11 +400,13 @@ export const MenuButtons = explicitConnect<OwnProps, StateProps, DispatchProps>(
abortFunction: getAbortFunction(state),
currentProfileUploadedInformation:
getCurrentProfileUploadedInformation(state),
getOverriddenZeroAtTimestamp: getOverriddenZeroAtTimestamp(state),
}),
mapDispatchToProps: {
dismissNewlyPublished,
revertToPrePublishedState,
profileRemotelyDeleted,
overrideZeroAt,
},
component: MenuButtonsImpl,
}
Expand Down
4 changes: 4 additions & 0 deletions src/components/shared/MarkerContextMenu.css
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,10 @@
background-image: url(../../../res/img/svg/end-selection-at-marker-end.svg);
}

.markerContextMenuIconOverrideZeroAtMarkerStart {
background-image: url(../../../res/img/svg/start-selection-at-marker-start.svg);
}

.markerContextMenuIconCopyDescription {
background-image: url(../../../res/img/svg/copy-dark.svg);
}
Expand Down
17 changes: 17 additions & 0 deletions src/components/shared/MarkerContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
setContextMenuVisibility,
updatePreviewSelection,
selectTrackFromTid,
overrideZeroAt,
} from 'firefox-profiler/actions/profile-view';
import {
getPreviewSelection,
Expand Down Expand Up @@ -65,6 +66,7 @@ type DispatchProps = {
readonly updatePreviewSelection: typeof updatePreviewSelection;
readonly setContextMenuVisibility: typeof setContextMenuVisibility;
readonly selectTrackFromTid: typeof selectTrackFromTid;
readonly overrideZeroAt: typeof overrideZeroAt;
};

type Props = ConnectedProps<OwnProps, StateProps, DispatchProps>;
Expand Down Expand Up @@ -141,6 +143,11 @@ class MarkerContextMenuImpl extends PureComponent<Props> {
});
};

overrideZeroAtMarkerStart = () => {
const { marker, overrideZeroAt } = this.props;
overrideZeroAt(marker.start);
};

_isZeroDurationMarker(marker: Marker | null): boolean {
return !marker || marker.end === null;
}
Expand Down Expand Up @@ -482,6 +489,15 @@ class MarkerContextMenuImpl extends PureComponent<Props> {
</>
)}

<div className="react-contextmenu-separator" />

<MenuItem onClick={this.overrideZeroAtMarkerStart}>
<span className="react-contextmenu-icon markerContextMenuIconOverrideZeroAtMarkerStart" />
<Localized id="MarkerContextMenu--override-zero-at-marker-start">
Override zero at marker’s start
</Localized>
</MenuItem>

<div className="react-contextmenu-separator" />
<MenuItem onClick={this.copyMarkerDescription}>
<span className="react-contextmenu-icon markerContextMenuIconCopyDescription" />
Expand Down Expand Up @@ -539,6 +555,7 @@ const MarkerContextMenu = explicitConnect<OwnProps, StateProps, DispatchProps>({
updatePreviewSelection,
setContextMenuVisibility,
selectTrackFromTid,
overrideZeroAt,
},
component: MarkerContextMenuImpl,
});
Expand Down
10 changes: 10 additions & 0 deletions src/reducers/profile-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -824,6 +824,15 @@ const mouseTimePosition: Reducer<Milliseconds | null> = (
}
};

const overrideZeroAt: Reducer<Milliseconds | null> = (state = null, action) => {
switch (action.type) {
case 'OVERRIDE_ZERO_AT':
return action.zeroAt;
default:
return state;
}
};

/**
* Provide a mechanism to wrap the reducer in a special function that can reset
* the state to the default values. This is useful when viewing multiple profiles
Expand Down Expand Up @@ -864,6 +873,7 @@ const profileViewReducer: Reducer<ProfileViewState> = wrapReducerInResetter(
hoveredMarker,
mouseTimePosition,
perTab: tableViewOptionsPerTab,
overrideZeroAt,
}),
profile,
globalTracks,
Expand Down
20 changes: 18 additions & 2 deletions src/selectors/profile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import * as Tracks from '../profile-logic/tracks';
import * as CPU from '../profile-logic/cpu';
import * as UrlState from './url-state';
import { ensureExists } from '../utils/types';
import { formatTimestamp } from '../utils/format-numbers';
import {
accumulateCounterSamples,
extractProfileFilterPageData,
Expand Down Expand Up @@ -96,8 +97,23 @@ export const getScrollToSelectionGeneration: Selector<number> = (state) =>
getProfileViewOptions(state).scrollToSelectionGeneration;
export const getFocusCallTreeGeneration: Selector<number> = (state) =>
getProfileViewOptions(state).focusCallTreeGeneration;
export const getZeroAt: Selector<Milliseconds> = (state) =>
getProfileRootRange(state).start;
export const getZeroAt: Selector<Milliseconds> = (state) => {
const viewOptions = getProfileViewOptions(state);
if (viewOptions.overrideZeroAt !== null) {
return viewOptions.overrideZeroAt;
}
return getProfileRootRange(state).start;
};
export const getOverriddenZeroAtTimestamp: Selector<string | null> = (
state
) => {
const viewOptions = getProfileViewOptions(state);
if (viewOptions.overrideZeroAt === null) {
return null;
}
const offset = viewOptions.overrideZeroAt - getProfileRootRange(state).start;
return formatTimestamp(offset);
};
export const getProfileTimelineUnit: Selector<TimelineUnit> = (state) => {
const { sampleUnits } = getProfile(state).meta;
return sampleUnits ? sampleUnits.time : 'ms';
Expand Down
17 changes: 17 additions & 0 deletions src/test/components/MarkerTable.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,23 @@ describe('MarkerTable', function () {
);
});

it('can override the zero at timing using the context menu', () => {
const { getRowElement } = setup();

const startNode = ensureExists(
getRowElement(/setTimeout/).querySelector('.start')
);
expect(startNode).toHaveTextContent('0.153s');

fireFullContextMenu(getRowElement(/setTimeout/) as HTMLElement);
fireFullClick(screen.getByText('Override zero at marker’s start'));

const startNode2 = ensureExists(
getRowElement(/setTimeout/).querySelector('.start')
);
expect(startNode2).toHaveTextContent('0s');
});

describe('EmptyReasons', () => {
it('shows reasons when a profile has no non-network markers', () => {
const { profile } = getProfileFromTextSamples('A'); // Just a simple profile without any marker.
Expand Down
14 changes: 14 additions & 0 deletions src/test/components/__snapshots__/MarkerChart.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,20 @@ exports[`MarkerChart context menus displays when right clicking on a marker 1`]
<div
class="react-contextmenu-separator"
/>
<div
aria-disabled="false"
class="react-contextmenu-item"
role="menuitem"
tabindex="-1"
>
<span
class="react-contextmenu-icon markerContextMenuIconOverrideZeroAtMarkerStart"
/>
Override zero at marker’s start
</div>
<div
class="react-contextmenu-separator"
/>
<div
aria-disabled="false"
class="react-contextmenu-item"
Expand Down
6 changes: 3 additions & 3 deletions src/test/components/__snapshots__/MenuButtons.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -2187,7 +2187,7 @@ exports[`app/MenuButtons <Publish> matches the snapshot for the menu buttons and
class="menuButtonsDownloadSize"
>
(
1.58 kB
1.59 kB
)
</span>
</a>
Expand Down Expand Up @@ -2305,7 +2305,7 @@ exports[`app/MenuButtons <Publish> matches the snapshot for the opened panel for
class="menuButtonsDownloadSize"
>
(
1.56 kB
1.57 kB
)
</span>
</a>
Expand Down Expand Up @@ -2418,7 +2418,7 @@ exports[`app/MenuButtons <Publish> matches the snapshot for the opened panel for
class="menuButtonsDownloadSize"
>
(
1.58 kB
1.59 kB
)
</span>
</a>
Expand Down
4 changes: 4 additions & 0 deletions src/types/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,10 @@ type UrlStateAction =
readonly type: 'CHANGE_MOUSE_TIME_POSITION';
readonly mouseTimePosition: Milliseconds | null;
}
| {
readonly type: 'OVERRIDE_ZERO_AT';
readonly zeroAt: Milliseconds | null;
}
| {
readonly type: 'CHANGE_TABLE_VIEW_OPTIONS';
readonly tab: TabSlug;
Expand Down
1 change: 1 addition & 0 deletions src/types/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export type ProfileViewState = {
hoveredMarker: MarkerReference | null;
mouseTimePosition: Milliseconds | null;
perTab: TableViewOptionsPerTab;
overrideZeroAt: number | null;
};
readonly profile: Profile | null;
globalTracks: GlobalTrack[];
Expand Down
14 changes: 13 additions & 1 deletion src/utils/format-numbers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,19 @@ export function formatTimestamp(
maxFractionalDigits: number = 3,
// precision is the minimum required precision.
precision: Milliseconds = Infinity
) {
): string {
if (time < 0) {
return (
'-' +
formatTimestamp(
Math.abs(time),
significantDigits,
maxFractionalDigits,
precision
)
);
}

if (precision !== Infinity) {
// Round the values to display nicer numbers when the extra precision
// isn't useful. (eg. show 3h52min10s instead of 3h52min14s)
Expand Down