1705684 - [Proton] The spacing is wrong in the Download details panel

Status: VERIFIED FIXED

(Firefox :: Downloads Panel, defect, P2)

Description

[Oana Botisan, Desktop QA]

Attachment: image (2).png

Affected versions

• Firefox 89.0a1

Affected platforms

• Windows 10 x64
• Ubuntu 18.04 x64
• macOS 10.15

Steps to reproduce

1. Go to https://www.thinkbroadband.com/download.
2. Start a download.
3. Openn the download panel and click on the element from the list.

Expected result

• There is a spacing between the subtitle and the rest of the text.
• There is no spacing between the two frases.

Actual result

• There is no spacing between the subtitle and the rest of the text.
• There is a spacing between the two frases.

Regression range

• This is not a regression.

Additional notes

• Please look at the attached image. This is from specs.

Comment 1

[Oana Botisan, Desktop QA]

Attachment: Screenshot_14.png

This is how it looks at the moment.

Comment 2

[Molly Howell (she/her) (no longer active)]

Hi Janice. I'm sending this to you because I've seen you taking other download panel questions, but feel free to redirect of course.

I have a couple of questions about the spec for the spacing in these download info panels. In Figma, most of the panels appear as in comment 0, but there's one that's laid out like comment 1 instead. This seems like an oversight to me; do you know if they should all be one or the other? Also, there's one panel in Figma that has the larger height, but with the buttons up against the text instead of against the bottom of the panel; is one of those correct for all the panels, or is that one different intentionally?

Comment 3

[Janice Cramer]

Hi Molly, I agree that it appears to be an oversight. I'll do some recon and get back to you asap.

Comment 4

[Janice Cramer]

(In reply to Molly Howell (she/her) [:mhowell] from comment #2)

Hi Janice. I'm sending this to you because I've seen you taking other download panel questions, but feel free to redirect of course.

I have a couple of questions about the spec for the spacing in these download info panels. In Figma, most of the panels appear as in comment 0, but there's one that's laid out like comment 1 instead. This seems like an oversight to me; do you know if they should all be one or the other?

The download panels should appear as in comment 0, not as in comment 1.

Notes:
– In the comment 0 screenshot, the heights of the default downloads view and the download details view are the same; this is so that the panel height does not shift but rather, stays stable.
– The comment 0 screenshot, the paragraph spacing appears to be 4 px, however I believe it should be 16 px. I noticed that in Figma, the download panel screens are inconsistent—some are 4 px and some are 16 px. The 16 px looks correct.
– Note that comment 1 screenshot, type size and style are also inaccurate. If you need more info here, let me know.

Also, there's one panel in Figma that has the larger height, but with the buttons up against the text instead of against the bottom of the panel; is one of those correct for all the panels, or is that one different intentionally?

The buttons should not be up against the text; they should be locked up with the bottom of the panel, like the others in Figma.

Comment 5

[Molly Howell (she/her) (no longer active)]

Thanks for the response! I do have one followup question.

(In reply to Janice Cramer from comment #4)

– In the comment 0 screenshot, the heights of the default downloads view and the download details view are the same; this is so that the panel height does not shift but rather, stays stable.

So the good news is, we're already maintaining the height when the panel view changes like that. In fact, that's the reason why the height looks so short in comment 1; the downloads list view only had one or two downloads in it, so the panel didn't have any room for the additional spacing. We would have to be adding a bunch more height in order to make it look like comment 0 in that case. Do you think we should be doing that, or keeping the shorter height?

– The comment 0 screenshot, the paragraph spacing appears to be 4 px, however I believe it should be 16 px. I noticed that in Figma, the download panel screens are inconsistent—some are 4 px and some are 16 px. The 16 px looks correct.

Okay, I can do that.

– Note that comment 1 screenshot, type size and style are also inaccurate. If you need more info here, let me know.

I think bug 1705685 is going to address that.

The buttons should not be up against the text; they should be locked up with the bottom of the panel, like the others in Figma.

Got it, thanks for clarifying.

Comment 6

[Janice Cramer]

See below:

(In reply to Molly Howell (she/her) [:mhowell] from comment #5)

Thanks for the response! I do have one followup question.

(In reply to Janice Cramer from comment #4)

– In the comment 0 screenshot, the heights of the default downloads view and the download details view are the same; this is so that the panel height does not shift but rather, stays stable.

So the good news is, we're already maintaining the height when the panel view changes like that. In fact, that's the reason why the height looks so short in comment 1; the downloads list view only had one or two downloads in it, so the panel didn't have any room for the additional spacing. We would have to be adding a bunch more height in order to make it look like comment 0 in that case. Do you think we should be doing that, or keeping the shorter height?

We should keep the shorter height.

– The comment 0 screenshot, the paragraph spacing appears to be 4 px, however I believe it should be 16 px. I noticed that in Figma, the download panel screens are inconsistent—some are 4 px and some are 16 px. The 16 px looks correct.

Okay, I can do that.

– Note that comment 1 screenshot, type size and style are also inaccurate. If you need more info here, let me know.

I think bug 1705685 is going to address that.

The buttons should not be up against the text; they should be locked up with the bottom of the panel, like the others in Figma.

Got it, thanks for clarifying.

Thank you, Molly!

Comment 7

[Molly Howell (she/her) (no longer active)]

Attachment: Bug 1705684 - Correct download details panel spacing. r=#desktop-theme-reviewers

Comment 8

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/b8575a6aead6

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Seems like a pretty low-risk uplift; did you want to nominate this for Beta & ESR91, Molly?

Comment 10

[Molly Howell (she/her) (no longer active)]

Comment on attachment 9237594 [details]
Bug 1705684 - Correct download details panel spacing. r=#desktop-theme-reviewers

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This is a Proton regression, a minor one to be sure but a very low-risk one to patch.
• User impact if declined: Cosmetic and readability issues on the download details panel.
• Fix Landed on Version: 93
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This is a very focused, cosmetic-only fix for a Proton styling bug.
• String or UUID changes made by this patch:

Beta/Release Uplift Approval Request

• User impact if declined:
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky):
• String changes made/needed:

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9237594 [details]
Bug 1705684 - Correct download details panel spacing. r=#desktop-theme-reviewers

Approved for 92.0b9 and 91.1esr.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr91/rev/62db48218c72

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/bc4ab51dc29b

Comment 14

[Oana Botisan, Desktop QA]

Verified the fix using Nightly 93.0a1, Firefox 92.0b9 and Firefox 91.1.0esr on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.15. The issue is not reproducing anymore.