Closed Bug 918782 Opened 11 years ago Closed 10 years ago

Move functional styles out of customizeMode.inc.css and panelUIOverlay.inc.css in browser/themes/shared/ to browser/base/

Categories

(Firefox :: Toolbars and Customization, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 29

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [Australis:P2][qa-])

Attachments

(1 file)

Because right now there's transitions in there which are required for correct functioning of the customization mode, meaning that'll break if people use custom themes.
Summary: Move functional styles out of themes/shared/customizableui/customizeMode.inc.css into browser/base/ included CSS → Move functional styles out of customizeMode.inc.css and panelUIOverlay.inc.css in browser/themes/shared/ to browser/base/
Let's get this done before we merge. The current state is kinda sadfaces for theme authors.
Whiteboard: [Australis:M?][Australis:P4] → [Australis:M?][Australis:P2]
(In reply to :Gijs Kruitbosch from comment #1)
> Let's get this done before we merge. The current state is kinda sadfaces for
> theme authors.

We didn't and ended up breaking customize mode for everyone using custom themes.
Whiteboard: [Australis:M?][Australis:P2] → [Australis:M?][Australis:P1]
Depends on: 940559
Assignee: nobody → gijskruitbosch+bugs
Blocks: 940559
No longer depends on: 940559
Let's start with customize mode. Mike de Boer is in the process of revamping all the panelUIOverlay.css styles in bug 878065 and it'd be annoying to have to deal with the conflicts there for either of us, so I'll wait with that until that bug has landed.
Attachment #8336006 - Flags: review?(mconley)
Status: NEW → ASSIGNED
Depends on: 878065
Comment on attachment 8336006 [details] [diff] [review]
[Australis] move functional customize mode styles to browser/base,

Works for me. Thanks Gijs!
Attachment #8336006 - Flags: review?(mconley) → review+
Comment on attachment 8336006 [details] [diff] [review]
[Australis] move functional customize mode styles to browser/base,

>+toolbarpaletteitem {
>+  transition: background-color, border-color, box-shadow, border-width;
>+  transition-duration: 10ms, 10ms, 10ms, 250ms;
>+  transition-timing-function: linear, linear, linear, ease-in-out;
>+}

>+toolbarpaletteitem[mousedown] {
>+  box-shadow: inset 0 0 3px hsl(204,100%,40%);
>+  cursor: -moz-grabbing;
>+  opacity: 0.8;
>+}

>+.panel-customization-placeholder,
>+toolbarpaletteitem[place="palette"],
>+toolbarpaletteitem[place="panel"] {
>+  transition: background-color, border-color, box-shadow, transform;
>+  transition-duration: 10ms, 10ms, 10ms, 250ms;
>+  transition-timing-function: linear, linear, linear, ease-in-out;
>+}
>+
>+toolbarpaletteitem[notransition][place="palette"],
>+toolbarpaletteitem[notransition][place="panel"] {
>+  transition: background-color, border-color, box-shadow;
>+  transition-duration: 10ms, 10ms, 10ms;
>+  transition-timing-function: linear, linear, linear;
>+}
>+
>+toolbarpaletteitem > toolbarbutton > .toolbarbutton-icon {
>+  transition: transform 50ms ease-in-out;
>+}
>+
>+toolbarpaletteitem[mousedown] > toolbarbutton > .toolbarbutton-icon {
>+  transform: scale(1.1);
>+}
>+
>+/* Override the toolkit styling for items being dragged over. */
>+toolbarpaletteitem[place="toolbar"] {
>+  border-left-width: 0;
>+  border-right-width: 0;
>+  margin-right: 0;
>+  margin-left: 0;
>+}

>+#customization-palette:not([hidden]) {
...
>+  margin-bottom: 25px;
>+}

How exactly is the above styling functional rather than presentational?
(In reply to Dão Gottwald [:dao] from comment #5)
> Comment on attachment 8336006 [details] [diff] [review]
> [Australis] move functional customize mode styles to browser/base,
> 
> >+toolbarpaletteitem {
> >+  transition: background-color, border-color, box-shadow, border-width;
> >+  transition-duration: 10ms, 10ms, 10ms, 250ms;
> >+  transition-timing-function: linear, linear, linear, ease-in-out;
> >+}
> 
> >+toolbarpaletteitem[mousedown] {
> >+  box-shadow: inset 0 0 3px hsl(204,100%,40%);
> >+  cursor: -moz-grabbing;
> >+  opacity: 0.8;
> >+}
> 
> >+.panel-customization-placeholder,
> >+toolbarpaletteitem[place="palette"],
> >+toolbarpaletteitem[place="panel"] {
> >+  transition: background-color, border-color, box-shadow, transform;
> >+  transition-duration: 10ms, 10ms, 10ms, 250ms;
> >+  transition-timing-function: linear, linear, linear, ease-in-out;
> >+}
> >+
> >+toolbarpaletteitem[notransition][place="palette"],
> >+toolbarpaletteitem[notransition][place="panel"] {
> >+  transition: background-color, border-color, box-shadow;
> >+  transition-duration: 10ms, 10ms, 10ms;
> >+  transition-timing-function: linear, linear, linear;
> >+}
> >+
> >+toolbarpaletteitem > toolbarbutton > .toolbarbutton-icon {
> >+  transition: transform 50ms ease-in-out;
> >+}
> >+
> >+toolbarpaletteitem[mousedown] > toolbarbutton > .toolbarbutton-icon {
> >+  transform: scale(1.1);
> >+}
> >+
> >+/* Override the toolkit styling for items being dragged over. */
> >+toolbarpaletteitem[place="toolbar"] {
> >+  border-left-width: 0;
> >+  border-right-width: 0;
> >+  margin-right: 0;
> >+  margin-left: 0;
> >+}
> 
> >+#customization-palette:not([hidden]) {
> ...
> >+  margin-bottom: 25px;
> >+}
> 
> How exactly is the above styling functional rather than presentational?

The margin-bottom for the customization palette I would agree isn't, so I'll move that back before landing. The rest is... complicated. I would argue the transform and border transitions for the drag feedback are functional, as they're about the drag affordance. Same for the mousedown/-moz-grabbing styling. I don't think it's reasonable to expect all third-party themes to copy that. Because of how CSS transition definitions work, AFAIK it's not possible to keep half the transitions (border-width, transform) in one place and the other half in the other, as the properties override each other and aren't "additive". Hence moving them completely.
(In reply to :Gijs Kruitbosch from comment #6)
> The margin-bottom for the customization palette I would agree isn't, so I'll
> move that back before landing. The rest is... complicated. I would argue the
> transform and border transitions for the drag feedback are functional, as
> they're about the drag affordance. Same for the mousedown/-moz-grabbing
> styling.

Drag affordance is presentational in my book.

> I don't think it's reasonable to expect all third-party themes to
> copy that.

That's true for a lot of new-UI styling and frankly their problem...

> Because of how CSS transition definitions work, AFAIK it's not
> possible to keep half the transitions (border-width, transform) in one place
> and the other half in the other, as the properties override each other and
> aren't "additive". Hence moving them completely.

Which of these transitions need to present at all for the script to function?
(In reply to Dão Gottwald [:dao] from comment #7)
> (In reply to :Gijs Kruitbosch from comment #6)
> > The margin-bottom for the customization palette I would agree isn't, so I'll
> > move that back before landing. The rest is... complicated. I would argue the
> > transform and border transitions for the drag feedback are functional, as
> > they're about the drag affordance. Same for the mousedown/-moz-grabbing
> > styling.
> 
> Drag affordance is presentational in my book.

I disagree, but I see how it's not super-clear-cut.

> > Because of how CSS transition definitions work, AFAIK it's not
> > possible to keep half the transitions (border-width, transform) in one place
> > and the other half in the other, as the properties override each other and
> > aren't "additive". Hence moving them completely.
> 
> Which of these transitions need to present at all for the script to function?

I'm not sure.

I really want to at least fix the customize mode transition, so I've landed with the bits that you disagree with left where they are, and we can discuss here about the rest.

remote:   https://hg.mozilla.org/integration/fx-team/rev/9e600f5128fe

This can go back to P2 now that that's landed.
Whiteboard: [Australis:M?][Australis:P1] → [Australis:P2][leave open]
No longer blocks: 940559
Attachment #8336006 - Flags: checkin+
Discussed this with Jared, we think we're actually done here. There's more cleanup to do in panelUIOverlay.inc.css, but it isn't about moving stuff to browser/base, so we won't do it here.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:P2][leave open] → [Australis:P2]
Target Milestone: --- → Firefox 29
Whiteboard: [Australis:P2] → [Australis:P2][qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: