Improve discoverability of downloading a local profile - #6216
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6216 +/- ##
=======================================
Coverage 83.69% 83.69%
=======================================
Files 348 348
Lines 37403 37433 +30
Branches 10498 10415 -83
=======================================
+ Hits 31303 31330 +27
- Misses 5673 5676 +3
Partials 427 427 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
01b9c86 to
3e094a0
Compare
|
Thanks, it's great that we have two different buttons now! It's a huge UX improvement over to the current state. I'm not so sure about the reduced size of the title and the left alignment being gone now. I think I would prefer to keep the text alignment but reducing the left margin, and reducing the size of the icon. I think I like it more when the title is larger. Currently it looks a bit too close to the subtitle below. But this might just be a personal preference so it might be good to get feedback from others. I was thinking maybe something like this: 
Reduced the icon size and margins, and reduced the title from css class 40 to 30. And these are the changes I made (on top of main, no this PR): diff --git a/src/components/app/MenuButtons/Publish.css b/src/components/app/MenuButtons/Publish.css
index 60429e8b08..2784ff8ff1 100644
--- a/src/components/app/MenuButtons/Publish.css
+++ b/src/components/app/MenuButtons/Publish.css
@@ -75,17 +75,19 @@
.publishPanelContent {
position: relative;
- /* This aligns all content, except the big icon. */
- padding-left: 70px;
+ /* This aligns all content, except the icon. */
+ padding-left: 34px;
}
.publishPanelTitle {
- /* "60px" This is the value to put the background image at the right location.
- * This background image is 44x44, so this puts it 16px left of the text. */
- padding-left: 60px;
- margin: 0 0 0 -60px;
+ /* "28px" This is the value to put the background image at the right location.
+ * The background image is scaled down to 18x18, so this puts it 10px left of
+ * the text. */
+ padding-left: 28px;
+ margin: 0 0 0 -28px;
background: var(--internal-info-icon) left center no-repeat;
- line-height: 44px; /* This is the height of the background image */
+ background-size: 18px 18px;
+ line-height: 28px;
}
.publishPanelInfoDescription {
diff --git a/src/components/app/MenuButtons/Publish.tsx b/src/components/app/MenuButtons/Publish.tsx
index b464465a33..5b4d0660b4 100644
--- a/src/components/app/MenuButtons/Publish.tsx
+++ b/src/components/app/MenuButtons/Publish.tsx
@@ -135,7 +135,7 @@
className="publishPanelContent photon-body-10"
onSubmit={this._onSubmit}
>
- <h1 className="publishPanelTitle photon-title-40">
+ <h1 className="publishPanelTitle photon-title-30">
{isRepublish ? (
<Localized id="MenuButtons--publish--reupload-performance-profile">
Re-upload Performance ProfileLet me know what you think! I guess these are all subjective though, so it might be good to ask opinions for others. cc @mstange @fqueze would be good if you have any opinions. |
|
@canova thanks for sharing your view! I am not quite sure what you mean by
It is still left aligned, isn't it? Do you mean the spacing below the icon (throughout the whole block) being gone? I think the size is reasonable, especially if we look at the titles of other menus, eg Profile Info. But I am happy to change it, if necessary. Let's indeed hear what others think. |
Ah, yeah I mean the left alignment of title and the rest of the text. For example: To me, this looks a bit cleaner in terms of the hierarchy of the panel. |
After series of talks and opinion shares with @canova and @fqueze, this version is now implemented! 😀 |
canova
left a comment
There was a problem hiding this comment.
Thanks, it's a clear usability improvement over the current one!
One thing I noticed is that the checkbox checked states are shared between those two. As a user, if I include some information for download, I might want to exclude them for upload. I think it would be better to split these states. I was even thinking of checking all of them by default for the download case (non-nightly release channels default to not sharing them by default). But I think it's good to look at that separately.
And an opposite things is the _inflightEncoding. I think we want to unify these inflight encodings so we can cancel them or prevent the double work from happening. The best way to do this is to keep this in the redux state. Maybe another phase like this:
| {
phase: 'ENCODING';
sanitizedProfile: Profile;
encodingPromise: Promise<ProfileEncodingResult>;
}Another thing I noticed is that, now that the download button is gone in the "upload" panel, we don't see the size of the profile anymore. I think I would like to see it still. For example, I was using that size to see how much of a difference it makes when I upload something with or without the time range etc. Not so sure if we should put it inside the "upload" button or if we should put it besides it or something. What do you think?
Downloading a profile was only reachable from inside the single "Upload Local Profile" menu, so users looking to save a profile to disk had no way to discover it from the toolbar. Replace that single menu with two buttons, "Download…" and "Share…", each with its own icon. Both open the same options panel; only the heading and the action button differ, so Download saves the profile to a file and Share uploads it for a shareable link. Closes firefox-devtools#3620
The two panels shared a single set of include/exclude checkboxes, so changing an option for one silently changed it for the other. Store the options per panel instead, defaulting both to the same values. The in-flight profile encoding now lives in the store too, so opening both panels (or pressing upload) reuses one compression instead of starting a second one for the same profile.
|
Thanks for the elaborate feedback and your suggestions, @canova! I've now addressed the issues. |
canova
left a comment
There was a problem hiding this comment.
It looks great to me! Thanks!!
Changes: [fatadel] Create the Network track from the timeline-network schema display location (#6224) [Markus Stange] Only call `getRawFrameTableBuilderWithExistingContents` once per symbolication batch. (#6233) [fatadel] Improve discoverability of downloading a local profile (#6216) [Nazım Can Altınova] Handle the cli daemon startup failures more gracefully with better errors (#6241) [Nazım Can Altınova] Add the ability to apply source maps from the CLI (#6229) [Nazım Can Altınova] Handle Text and Log marker payloads with their marker schema (#6247) [Nazım Can Altınova] Bump the Gecko profile version to make sure that the Text and Log marker changes are picked up in the frontends (#6252) [fatadel] Deactivate a menu button as soon as its panel is dismissed (#6251) [Nazım Can Altınova] 🔃 Sync: l10n -> main (August 10, 2026) (#6253) [Nazım Can Altınova] Bump profiler-cli version to 0.8.0 (#6254) And special thanks to our localizers: de: Ger de: Michael Köhler el: George kitsoukakis en-CA: chutten en-CA: Saurabh en-GB: Ian Neal es-CL: ravmn fy-NL, nl: Fjoerfoks fr: Théo Chevalier fy-NL: Fjoerfoks ia: Melo46 it: Francesco Lodolo [:flod] nl: Fjoerfoks ru: michellemelsspam ru: Valery Ledovskoy tr: giray tr: Selim Şumlu zh-TW: Pin-guang Chen
Main | Deploy preview
Downloading a profile was only reachable from inside the single "Upload Local Profile" menu, so users looking to save a profile to disk had no way to discover it from the toolbar.
Replace that single menu with two buttons, "Download…" and "Share…",each with its own icon. Both open the same options panel; only theheading and the action button differ, so Download saves the profile toa file and Share uploads it for a shareable link.
Closes #3620
Profile
This is how it looks like.