remove padding from social panels

RESOLVED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: mixedpuppy, Assigned: mixedpuppy)

Tracking

Trunk
Firefox 18
Points:
---

Firefox Tracking Flags

(firefox17 fixed)

Details

(Whiteboard: [Fx17][qa-])

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

5 years ago
Created attachment 655644 [details]
panel.png

We need to consider if we want to customize the panel styling in any way, for both status panels and the flyout panels.  Right now we use the standard css for the panels.  IMO the primary issue is the margin.
(Assignee)

Updated

5 years ago
Whiteboard: [Fx17]
I don't think this is an issue. Providers can customize their styling to either match the background color, or place a border around their content.

I would vote for RESO-WONTFIX.
The default here should really be Firefox's styling for regular arrow notification panels, such as you'd see in permission notification and the Edit this Bookmark panel.  We get some benefits by keeping the panel consistent - it's a visual assurance to that the notification, while it contains social network information, is in fact from Firefox and can be trusted.  I'm not sure I see the need to customize it, especially in v1.

(In reply to Shane Caraveo (:mixedpuppy) from comment #0)
> IMO the primary issue is the margin.

Is your concern that the margin around the panel may be too large for a provider's content?
(Assignee)

Comment 3

5 years ago
(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #2)

> (In reply to Shane Caraveo (:mixedpuppy) from comment #0)
> > IMO the primary issue is the margin.
> 
> Is your concern that the margin around the panel may be too large for a
> provider's content?

Your initial designs did not have a margin. If the wide margin is ok, this can be closed.
(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> (In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #2)
> 
> > (In reply to Shane Caraveo (:mixedpuppy) from comment #0)
> > > IMO the primary issue is the margin.
> > 
> > Is your concern that the margin around the panel may be too large for a
> > provider's content?
> 
> Your initial designs did not have a margin. If the wide margin is ok, this
> can be closed.

I just double-checked, and you're very right.  Attachments like the first one on Bug 771826 do have a smaller margin than our standard panels - in fact it's just 10 pixels!  Thanks for noticing - let's indeed customize the styling to decrease the margin here.
Shane & I synced up on IRC.  While it's not strictly consistent with other arrow panel notifications, we're going to have essentially no margin on the arrow panel, but with a small buffer margin in the direction of the arrow (at the top for menu panels and at the right for overlays) so that the panel's content does not interfere with the arrow.

What we get with this approach is a wide range of freedom in rendering content.  Social networks can always add a margin on a panel, but they can't remove one.  The margin-less approach lets them decide which layout is best for their individual content.  For instance, if a network wants to create a panel that behaves more like a menu with list items, a margin would not look as good as it does in the Firefox static content panels (like permission dialogs) that it was originally designed for for.  This is the safest approach and one that gives social networks the flexibility their content will need.
(Assignee)

Comment 6

5 years ago
Created attachment 657476 [details] [diff] [review]
no padding.patch

per conversations with Borris, we're going no-padding
Assignee: jboriss → mixedpuppy
Attachment #655644 - Attachment is obsolete: true
Attachment #657476 - Flags: review?(dao)
Comment on attachment 657476 [details] [diff] [review]
no padding.patch

>+#social-notification-panel .panel-arrowcontent {

Use the child selector.

>+#social-notification-box iframe {

This iframe should probably have an id or class.
Attachment #657476 - Flags: review?(dao) → review-
(Assignee)

Comment 8

5 years ago
Created attachment 658208 [details] [diff] [review]
no padding.patch
Attachment #657476 - Attachment is obsolete: true
Attachment #658208 - Flags: review?(jaws)
Attachment #658208 - Flags: review?(dao)
(Assignee)

Updated

5 years ago
Attachment #658208 - Flags: review?(jaws) → review?(felipc)
Comment on attachment 658208 [details] [diff] [review]
no padding.patch

Review of attachment 658208 [details] [diff] [review]:
-----------------------------------------------------------------

the border-radius value is different on each platform, and that number is also prone to go out of sync. I tested and if you set the border-radius to inherit on both the box#social-notification-box and the iframe, it should work
(Assignee)

Comment 10

5 years ago
Created attachment 659035 [details] [diff] [review]
no padding.patch
Attachment #658208 - Attachment is obsolete: true
Attachment #658208 - Flags: review?(felipc)
Attachment #658208 - Flags: review?(dao)
Attachment #659035 - Flags: review?(felipc)
Attachment #659035 - Flags: review?(dao)
Attachment #659035 - Flags: review?(felipc) → review+

Updated

5 years ago
Attachment #659035 - Flags: review?(dao) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/e0b1ba698e11
OS: Mac OS X → All
Summary: panel styling → remove padding from social panels
(Assignee)

Comment 12

5 years ago
Comment on attachment 659035 [details] [diff] [review]
no padding.patch

[Approval Request Comment]
correct styling for panels in social
Attachment #659035 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/e0b1ba698e11
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 18
Attachment #659035 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/11477e0712a0
status-firefox17: --- → fixed
Hardware: x86 → All
Version: unspecified → Trunk
Flagging [qa-] as I don't think this needs QA verification. Please correct me if I am wrong.
Whiteboard: [Fx17] → [Fx17][qa-]
You need to log in before you can comment on or make changes to this bug.