Discovery pane has extra left padding

VERIFIED FIXED in Firefox 50

Status

()

defect
P4
normal
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: mstriemer, Assigned: Paenglab)

Tracking

unspecified
mozilla50
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox50 verified)

Details

(Whiteboard: triaged)

Attachments

(2 attachments)

The old discovery pane needed some left padding but the new one doesn't. The left padding should be removed from the discovery section of the add-ons manager.

The attached image shows the padding with a different background colour.
I'm pretty sure this comes from here:
https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/in-content/common.inc.css#89

So it isn't specific to the discovery pane, it is part of the main-content class which sounds like something pretty common.  I'm not sure why that padding is there or what an appropriate fix is here.

This ni? is a little bit of a shot in the dark but I'm lost on this front-end styling stuff, if you're not the right person maybe you can redirect to a better candidate?  (for background for anybody coming in who isn't familiar with add-ons jargon, "add-ons manager" is the page with the url "about:addons" and "discovery pane" is the main panel displayed in the add-ons manager when "get add-ons" is selected in the left-hand sidebar)
Flags: needinfo?(gijskruitbosch+bugs)
Feels like we should override that start padding along with the end padding and so on here:

https://dxr.mozilla.org/mozilla-central/rev/16663eb3dcfa759f25b5e27b101bc79270c156f2/toolkit/themes/shared/extensions/extensions.inc.css#8-12

and then re-add the start padding on the pages where we need it (such as the list view, and maybe others).

Richard, you did the restyling in bug 989469 - does this sound right to you?

Andrew: how do we see this new discovery pane? I still get the "old" one when using about:addons in Nightly. Should we be removing the padding for the "old" one as well? It seems like we could, but because of comment #0 I'm not sure that's right, can you clarify?
Flags: needinfo?(richard.marti)
Flags: needinfo?(gijskruitbosch+bugs)
Flags: needinfo?(aswan)
The question could be, what is simpler, remove the padding from Add-on manager or adapt the "new" discovery pane to this padding.

The in-content design wants a padding around the content of 48px. The asymmetrical padding was done to move the scrollbar to the edge and not inside the page. Yes, the padding could be removed by setting #main-content { padding: 0;} and then add this padding-inline-sart: 48px; to https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/extensions/extensions.inc.css#19 respective margin-inline-start: 48px; to https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/extensions/extensions.inc.css#234 and https://dxr.mozilla.org/mozilla-central/source/toolkit/themes/shared/extensions/extensions.inc.css#651. There may be other places but this three are for sure.
Flags: needinfo?(richard.marti)
(In reply to :Gijs Kruitbosch from comment #2)
> Andrew: how do we see this new discovery pane? I still get the "old" one
> when using about:addons in Nightly. Should we be removing the padding for
> the "old" one as well? It seems like we could, but because of comment #0 I'm
> not sure that's right, can you clarify?

About using the new pane, in theory you can change the "extensions.webservice.discoverURL" preference to point to "https://discovery.addons.allizom.org", though I just tried that and it didn't work, Stuart tells me it is due to CSP.

For the other questions, I'm not really sure, maybe Mark can help?
Flags: needinfo?(scolville)
Flags: needinfo?(mstriemer)
Flags: needinfo?(aswan)
The CSP issues are tracked here: https://github.com/mozilla/addons-frontend/issues/433
Flags: needinfo?(scolville)
And one last update to "how to use the new disco pane": CSP fixes have been done in the dev environment, so setting the preference extensions.webservice.discoverURL to https://discovery.addons-dev.allizom.org/ in nightly should do it.  At some point that change will go to stage and https://discovery.addons.allizom.org/ will also work.
We could add some matching margin to the disco pane fairly easily. We thought it would be nice to avoid tying our stylesheets together at this point if possible but this isn't too bad.
Flags: needinfo?(mstriemer)
Priority: -- → P4
Whiteboard: triaged
In triage we spoke about the temporary least worst solution being to add some padding to the right of the page. It ties the two templates together and at some point we'll end up with a bug that says "if you open the discovery pane outside of about:addons there is right padding but no left padding". But at this point that might be quickest work around.
(In reply to Andy McKay [:andym] from comment #8)
> In triage we spoke about the temporary least worst solution being to add
> some padding to the right of the page. It ties the two templates together
> and at some point we'll end up with a bug that says "if you open the
> discovery pane outside of about:addons there is right padding but no left
> padding". But at this point that might be quickest work around.

That fix was committed to the disco pane here: https://github.com/mozilla/addons-frontend/commit/6313f309e065343d6318673f83c916e61ab49075
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
We experience problems with the workaround we found for left padding, would be great to get this padding removed.

Users will have no way of knowing why scrolling does not work when there mouse is on the left side of the page, close to the visual edge.
https://github.com/mozilla/addons-frontend/issues/602

And we'd like to dim the page while a user watches the video for it to be easier to focus on it.
https://github.com/mozilla/addons-frontend/issues/601

Both will not be possible as long as we have that padding.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I would likely make a huge hash of this.  Richard, based on your remarks in comment 3 could you take this one?
Flags: needinfo?(richard.marti)
This should fix all asymmetric margins/paddings.
Assignee: aswan → richard.marti
Status: REOPENED → ASSIGNED
Flags: needinfo?(richard.marti)
Attachment #8763130 - Flags: review?(gijskruitbosch+bugs)
(In reply to Richard Marti (:Paenglab) from comment #12)
> Created attachment 8763130 [details] [diff] [review]
> Bug1273974.patch
> 
> This should fix all asymmetric margins/paddings.

Does this mean the discovery pane in about:addons will have 48px either side of the iframe?

For the disco pane we wanted to remove all the margin/padding so that the iframe can fill the available space next to the sidebar nav and as a result the scrolling of the iframe content is possible when over any part of the page that's not the sidebar.
Flags: needinfo?(richard.marti)
No, the disco pane should have 0px.
Flags: needinfo?(richard.marti)
Attachment #8763130 - Flags: review?(gijskruitbosch+bugs) → review+
Keywords: checkin-needed
Pushed by cbook@mozilla.com:
https://hg.mozilla.org/integration/fx-team/rev/535a13bd2bf9
Remove the asymmetric margin from Add-on manager content. r=Gijs
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/535a13bd2bf9
Status: ASSIGNED → RESOLVED
Closed: 3 years ago3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
Flags: qe-verify+
I was able to reproduce the initial issue on Firefox 49.0a1 (2016-05-18) under Windows 10 64-bit.

Verified fixed on Firefox 52.0a1 (2016-09-28/29), Firefox 51.0a2 (2016-09-28/29) and Firefox 50.0b2 (20160926162149) under Windows 10 64-bit, Ubuntu 16.04 32-bit and Mac OS X 10.10.4.
The extra left padding is no longer displayed in new disco pane.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.