Skip to content
Merged
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
2 changes: 2 additions & 0 deletions locales/en-US/app.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ MenuButtons--metaInfo--attempting-resymbolicate = Attempting to re-symbolicate p
MenuButtons--metaInfo--currently-symbolicating = Currently symbolicating profile
MenuButtons--metaInfo--cpu = CPU:
MenuButtons--metaInfo--main-memory = Main memory:
MenuButtons--index--show-moreInfo-button = Show more
MenuButtons--index--hide-moreInfo-button = Show less

# This string is used when we have the information about both physical and
# logical CPU cores.
Expand Down
18 changes: 10 additions & 8 deletions res/css/photon/button.css
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
opacity: 0.4;
}

.photon-button:hover:enabled {
/* Note: we're using :not(:disabled) instead of :enabled so that this can apply
* to non-input elements too. */
.photon-button:hover:not(:disabled) {
background-color: var(--grey-90-a20);
}

.photon-button:hover:active:enabled {
.photon-button:hover:active:not(:disabled) {
background-color: var(--grey-90-a30);
}

Expand All @@ -50,11 +52,11 @@
color: #fff;
}

.photon-button-primary:hover:enabled {
.photon-button-primary:hover:not(:disabled) {
background-color: var(--blue-70);
}

.photon-button-primary:hover:active:enabled {
.photon-button-primary:hover:active:not(:disabled) {
background-color: var(--blue-80);
}

Expand All @@ -64,11 +66,11 @@
color: #fff;
}

.photon-button-destructive:hover:enabled {
.photon-button-destructive:hover:not(:disabled) {
background-color: var(--red-70);
}

.photon-button-destructive:hover:active:enabled {
.photon-button-destructive:hover:active:not(:disabled) {
background-color: var(--red-80);
}

Expand All @@ -78,11 +80,11 @@
background-color: transparent;
}

.photon-button-ghost:hover:enabled {
.photon-button-ghost:hover:not(:disabled) {
background-color: var(--grey-90-a10);
}

.photon-button-ghost:hover:active:enabled {
.photon-button-ghost:hover:active:not(:disabled) {
background-color: var(--grey-90-a20);
}

Expand Down
22 changes: 21 additions & 1 deletion src/components/app/MenuButtons/MetaInfo.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@

.metaInfoLargeContent {
overflow: scroll;
height: 100px;
max-height: 100px;
overflow-wrap: anywhere;
}

Expand Down Expand Up @@ -67,3 +67,23 @@
font-size: 1.2em;
font-weight: 700;
}

.moreInfoSummary {
display: flex;
align-items: center;
justify-content: center;
margin-top: 24px;
user-select: none;
}

.moreInfoSummary::marker {
content: '';
}

.moreInfoPart {
margin-top: 15px;
}

.moreInfoValue {
overflow-x: scroll;
}
70 changes: 68 additions & 2 deletions src/components/app/MenuButtons/MetaInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,10 @@ import { MetaOverheadStatistics } from './MetaOverheadStatistics';
import {
getProfile,
getSymbolicationStatus,
getProfileExtraInfo,
} from 'firefox-profiler/selectors/profile';
import { resymbolicateProfile } from 'firefox-profiler/actions/receive-profile';
import { formatFromMarkerSchema } from 'firefox-profiler/profile-logic/marker-schema';

import {
formatBytes,
Expand All @@ -25,14 +27,23 @@ import {
import { assertExhaustiveCheck } from 'firefox-profiler/utils/flow';
import explicitConnect from 'firefox-profiler/utils/connect';

import type { Profile, SymbolicationStatus } from 'firefox-profiler/types';
import type {
Profile,
SymbolicationStatus,
ExtraProfileInfoSection,
} from 'firefox-profiler/types';
import type { ConnectedProps } from 'firefox-profiler/utils/connect';

import './MetaInfo.css';

type State = {|
showsMoreInfo: boolean,
|};

type StateProps = $ReadOnly<{|
profile: Profile,
symbolicationStatus: SymbolicationStatus,
profileExtraInfo: ExtraProfileInfoSection[],
|}>;

type DispatchProps = $ReadOnly<{|
Expand All @@ -44,7 +55,9 @@ type Props = ConnectedProps<{||}, StateProps, DispatchProps>;
/**
* This component formats the profile's meta information into a dropdown panel.
*/
class MetaInfoPanelImpl extends React.PureComponent<Props> {
class MetaInfoPanelImpl extends React.PureComponent<Props, State> {
state = { showsMoreInfo: false };

/**
* This method provides information about the symbolication status, and a button
* to re-trigger symbolication.
Expand Down Expand Up @@ -120,6 +133,55 @@ class MetaInfoPanelImpl extends React.PureComponent<Props> {
}
}

_handleMoreInfoButtonClick = (event: SyntheticMouseEvent<>) => {
event.preventDefault();
this.setState((state) => ({ showsMoreInfo: !state.showsMoreInfo }));
Comment thread
parttimenerd marked this conversation as resolved.
};

_renderMoreInfoSection(section: ExtraProfileInfoSection) {
return (
<div key={section.label}>
Comment thread
parttimenerd marked this conversation as resolved.
<h2 className="metaInfoSubTitle">{section.label}</h2>
<div className="metaInfoSection">
{section.entries.map(({ label, format, value }) => {
return (
<div className="moreInfoRow" key={label}>
<span className="metaInfoWideLabel">{label}</span>
<div className="moreInfoValue">
{formatFromMarkerSchema('moreInfo', format, value)}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The same comment applies here: this isn't a marker so this doesn't look appropriate. However I realize that formatFromMarkerSchema is more generic than just for markers and could probably move to another file (src/utils/format-value.js for example) and be renamed (formatValue).
I would prever that we do it now, in a separate PR, and then rebase this patch on top of it.
However if that's too much work for you now, I think we can go in this way and file a follow-up issue.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

As explained before, I'm totally for the changes, but I think they are best in a "Factor out marker formatting" PR which I'll submit after the admission of #4201, if that's ok.

</div>
</div>
);
})}
</div>
</div>
);
}

_renderMoreInfo() {
return (
<details className="metaInfoRow" open={this.state.showsMoreInfo}>
Comment thread
parttimenerd marked this conversation as resolved.
<summary
className="moreInfoSummary photon-button photon-button-default photon-button-micro"
onClick={this._handleMoreInfoButtonClick}
>
<Localized
id={
this.state.showsMoreInfo
? `MenuButtons--index--hide-moreInfo-button`
: `MenuButtons--index--show-moreInfo-button`
}
>
{this.state.showsMoreInfo ? 'Show Less' : 'Show More'}
</Localized>
</summary>
<div className="moreInfoPart">
{this.props.profileExtraInfo.map(this._renderMoreInfoSection)}
</div>
</details>
);
}

render() {
const { meta, profilerOverhead } = this.props.profile;
const { configuration } = meta;
Expand Down Expand Up @@ -403,6 +465,9 @@ class MetaInfoPanelImpl extends React.PureComponent<Props> {
{profilerOverhead ? (
<MetaOverheadStatistics profilerOverhead={profilerOverhead} />
) : null}
{this.props.profileExtraInfo.length !== 0
? this._renderMoreInfo()
: null}
</>
);
}
Expand Down Expand Up @@ -444,6 +509,7 @@ export const MetaInfoPanel = explicitConnect<{||}, StateProps, DispatchProps>({
mapStateToProps: (state) => ({
profile: getProfile(state),
symbolicationStatus: getSymbolicationStatus(state),
profileExtraInfo: getProfileExtraInfo(state),
}),
mapDispatchToProps: {
resymbolicateProfile,
Expand Down
4 changes: 4 additions & 0 deletions src/selectors/profile.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ import type {
MarkerSchemaByName,
SampleUnits,
IndexIntoSamplesTable,
ExtraProfileInfoSection,
} from 'firefox-profiler/types';

export const getProfileView: Selector<ProfileViewState> = (state) =>
Expand Down Expand Up @@ -924,3 +925,6 @@ export const getProfileUsesMultipleStackTypes: Selector<boolean> = (state) => {
}
return profile.meta.usesOnlyOneStackType !== true;
};

export const getProfileExtraInfo: Selector<ExtraProfileInfoSection[]> =
createSelector(getProfile, (profile) => profile.meta.extra || []);
51 changes: 51 additions & 0 deletions src/test/components/MenuButtons.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,6 +517,57 @@ describe('app/MenuButtons', function () {
expect(getMetaInfoPanel()).toMatchSnapshot();
});

it('with no extra info there is no more info button', async () => {
Comment thread
parttimenerd marked this conversation as resolved.
const { profile } = getProfileFromTextSamples('A');
const { displayMetaInfoPanel } = await setupForMetaInfoPanel(profile);
await displayMetaInfoPanel();

expect(screen.queryByText('Show more')).not.toBeInTheDocument();
});

it('with more extra info, opens more info section if clicked', async () => {
const { profile } = getProfileFromTextSamples('A');
profile.meta.extra = [
{
label: 'CPU',
entries: [
{
label: 'CPU 1',
format: 'string',
value: 'Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz',
},
],
},
{
label: 'Memory',
entries: [],
},
{
label: 'Hard Drives',
entries: [
{
label: 'SSD',
format: 'string',
value: 'Samsung SSD 850 EVO 500GB',
},
{
label: 'HDD',
format: 'string',
value: 'Seagate ST1000LM035-1RK172',
},
],
},
];

const { displayMetaInfoPanel } = await setupForMetaInfoPanel(profile);
await displayMetaInfoPanel();

const summary = screen.getByText('Show more');
fireFullClick(summary);
const moreInfoPart = document.querySelector('.moreInfoPart');
expect(moreInfoPart).toMatchSnapshot();
});

describe('deleting a profile', () => {
const FAKE_HASH = 'FAKE_HASH';
const FAKE_PROFILE_DATA = {
Expand Down
81 changes: 81 additions & 0 deletions src/test/components/__snapshots__/MenuButtons.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1132,6 +1132,87 @@ exports[`app/MenuButtons <MetaInfoPanel> matches the snapshot with device inform
</div>
`;

exports[`app/MenuButtons <MetaInfoPanel> with more extra info, opens more info section if clicked 1`] = `
<div
class="moreInfoPart"
>
<div>
<h2
class="metaInfoSubTitle"
>
CPU
</h2>
<div
class="metaInfoSection"
>
<div
class="moreInfoRow"
>
<span
class="metaInfoWideLabel"
>
CPU 1
</span>
<div
class="moreInfoValue"
>
Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
</div>
</div>
</div>
</div>
<div>
<h2
class="metaInfoSubTitle"
>
Memory
</h2>
<div
class="metaInfoSection"
/>
</div>
<div>
<h2
class="metaInfoSubTitle"
>
Hard Drives
</h2>
<div
class="metaInfoSection"
>
<div
class="moreInfoRow"
>
<span
class="metaInfoWideLabel"
>
SSD
</span>
<div
class="moreInfoValue"
>
Samsung SSD 850 EVO 500GB
</div>
</div>
<div
class="moreInfoRow"
>
<span
class="metaInfoWideLabel"
>
HDD
</span>
<div
class="moreInfoValue"
>
Seagate ST1000LM035-1RK172
</div>
</div>
</div>
</div>
</div>
`;

exports[`app/MenuButtons <MetaInfoPanel> with no statistics object should not make the app crash 1`] = `
<div
class="arrowPanel open metaInfoPanel"
Expand Down
Loading