Closed Bug 1381554 Opened 3 years ago Closed 3 years ago

Remove rounded corners on all doorhangers

Categories

(Toolkit :: Themes, enhancement)

56 Branch
enhancement
Not set

Tracking

()

RESOLVED WONTFIX

People

(Reporter: daniela, Assigned: daniela)

References

Details

(Whiteboard: [ux] [photon])

Attachments

(2 files)

+++ This bug was initially created as a clone of Bug #1374339 +++

Remove rounded corners on all doorhangers (see #2 in attachment)
Component: Menus → Themes
Product: Firefox → Toolkit
Comment on attachment 8887128 [details]
Bug 1381554 - Remove rounded corners in doorhangers.

Because this is a global theme change, I've moved the bug to the Toolkit product, and I've redirected the review to Dão.
Attachment #8887128 - Flags: review?(paolo.mozmail) → review?(dao+bmo)
Comment on attachment 8887128 [details]
Bug 1381554 - Remove rounded corners in doorhangers.

https://reviewboard.mozilla.org/r/157878/#review162958

::: toolkit/themes/osx/global/popup.css
(Diff revision 1)
>  }
>  
>  .panel-arrowcontent {
>    -moz-appearance: none;
>    background: var(--arrowpanel-background);
> -  border-radius: var(--arrowpanel-border-radius);

Please completely remove that CSS variable rather than just this reference to it.

::: toolkit/themes/osx/global/popup.css
(Diff revision 1)
>  }
>  
>  .panel-arrowcontent {
>    -moz-appearance: none;
>    background: var(--arrowpanel-background);
> -  border-radius: var(--arrowpanel-border-radius);

Presumably you'll want to remove this too:
http://searchfox.org/mozilla-central/rev/01d27fdd3946f7210da91b18fcccca01d7324fe2/toolkit/themes/windows/global/popup.css#60-68
Attachment #8887128 - Flags: review?(dao+bmo) → review-
Comment on attachment 8887128 [details]
Bug 1381554 - Remove rounded corners in doorhangers.

https://reviewboard.mozilla.org/r/157878/#review163410

::: addon-sdk/source/lib/sdk/panel/utils.js:258
(Diff revision 2)
>    frame.setAttribute("transparent", "transparent");
>    frame.setAttribute("autocompleteenabled", true);
>    frame.setAttribute("tooltip", "aHTMLTooltip");
>    if (platform === "darwin") {
> -    frame.style.borderRadius = "var(--arrowpanel-border-radius, 3.5px)";
>      frame.style.padding = "1px";

Should probably remove the Mac-specific padding too at this point?

::: browser/themes/shared/customizableui/customizeMode.inc.css
(Diff revision 2)
>    background-clip: padding-box;
>    border: 1px solid var(--arrowpanel-border-color);
>    box-shadow: 0 0 10px hsla(0,0%,0%,.2);
>  %ifdef XP_MACOSX
>    -moz-appearance: none;
> -  border-radius: var(--arrowpanel-border-radius);

I believe -moz-appearance: none; can go away too
Attachment #8887128 - Flags: review?(dao+bmo) → review+
Blocks: 1374339
Is this ready to land?
Flags: needinfo?(darcese)
(In reply to Dão Gottwald [::dao] from comment #7)
> Is this ready to land?

Yes!
Flags: needinfo?(darcese)
Keywords: checkin-needed
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cf72ce758a61
Remove rounded corners in doorhangers. r=dao
Keywords: checkin-needed
We're sorry - something has gone wrong while rewriting or rebasing your commits. The commits being pushed no longer match what was requested. Please file a bug.
Uh, this should have gotten UI-review. We used square corners for the new hamburger panel on osx and were then told that that was wrong - see bug 1374315.

Stephen, please clarify what the intent here is, where these specs come from, and what the right thing is.
Flags: needinfo?(shorlander)
We got this spec from :amylee, originally from this: Bug #1374339, so the UI-R has been looked at. We are happy to either drop it or fix the failing test depending on what Amy and/or Stephen decide.
Flags: needinfo?(amlee)
(In reply to Erica from comment #13)
> We got this spec from :amylee, originally from this: Bug #1374339, so the
> UI-R has been looked at. We are happy to either drop it or fix the failing
> test depending on what Amy and/or Stephen decide.

I had reported removing rounded corners on other door-hangers after seeing seeing the hamburger menu landing in Nightly with no rounded corners so visually there was a mix of rounded and non-rounded corners on door-hangers in OSX. I took a look at bug 1374315 and it looks like the photon hamburger menu was implemented incorrectly to begin with which I was not aware of. In this case, please keep the rounded corners since it looks like that was the original intent. Apologies for the confusion.
Flags: needinfo?(amlee)
(In reply to Amy Lee [:amylee] UX from comment #14)
> (In reply to Erica from comment #13)
> > We got this spec from :amylee, originally from this: Bug #1374339, so the
> > UI-R has been looked at. We are happy to either drop it or fix the failing
> > test depending on what Amy and/or Stephen decide.
> 
> I had reported removing rounded corners on other door-hangers after seeing
> seeing the hamburger menu landing in Nightly with no rounded corners so
> visually there was a mix of rounded and non-rounded corners on door-hangers
> in OSX. I took a look at bug 1374315 and it looks like the photon hamburger
> menu was implemented incorrectly to begin with which I was not aware of. In
> this case, please keep the rounded corners since it looks like that was the
> original intent. Apologies for the confusion.

Yes, the corners on macOS should still be rounded. I thought there was a separate bug for that, but I can't find it.
Flags: needinfo?(shorlander)
(In reply to Stephen Horlander [:shorlander] from comment #15)
> (In reply to Amy Lee [:amylee] UX from comment #14)
> > (In reply to Erica from comment #13)
> > > We got this spec from :amylee, originally from this: Bug #1374339, so the
> > > UI-R has been looked at. We are happy to either drop it or fix the failing
> > > test depending on what Amy and/or Stephen decide.
> > 
> > I had reported removing rounded corners on other door-hangers after seeing
> > seeing the hamburger menu landing in Nightly with no rounded corners so
> > visually there was a mix of rounded and non-rounded corners on door-hangers
> > in OSX. I took a look at bug 1374315 and it looks like the photon hamburger
> > menu was implemented incorrectly to begin with which I was not aware of. In
> > this case, please keep the rounded corners since it looks like that was the
> > original intent. Apologies for the confusion.
> 
> Yes, the corners on macOS should still be rounded. I thought there was a
> separate bug for that, but I can't find it.

bug 1374315 is for updating the hamburger panel to have rounded corners. I guess that means we should close this as wontfix?
Status: NEW → RESOLVED
Closed: 3 years ago
Flags: needinfo?(darcese)
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.