The default bug view has changed. See this FAQ.

Make the Highlighter Toolbar look like shorlander's design

RESOLVED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: paul, Assigned: paul)

Tracking

Trunk
Firefox 10
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [minotaur])

Attachments

(2 attachments, 3 obsolete attachments)

(Assignee)

Description

6 years ago
We may want to use OSX design (bug 676253) or Windows design (Bug 676255).
(Assignee)

Comment 1

6 years ago
Created attachment 557581 [details] [diff] [review]
patch v1 WIP
Assignee: nobody → paul
Status: NEW → ASSIGNED
Created attachment 557619 [details]
patch v1 - screenshot - ubuntu 10.04

This is Ubuntu 10.04 LTS, clearlooks theme, gnome 2.30, gtk2.

The various Inspect button states:

http://img.i7m.de/show/nxwuk.png

Third state, mouse over, does not look too good for now.
(Assignee)

Comment 3

6 years ago
Thank you Mihai.
(Assignee)

Updated

6 years ago
Whiteboard: [minotaur]
(Assignee)

Updated

6 years ago
Blocks: 672006
(Assignee)

Comment 4

6 years ago
Created attachment 559097 [details] [diff] [review]
patch v1

Based on Mac design (bug 676253).
Attachment #557581 - Attachment is obsolete: true
Attachment #557619 - Attachment is obsolete: true
Attachment #559097 - Flags: review?(dao)
Comment on attachment 559097 [details] [diff] [review]
patch v1

>+#inspector-toolbar {
>+  -moz-appearance: none;
>+  height: 32px;

Why are you setting a height here rather than letting the toolbar be sized automatically based on its contents?

>+#inspector-inspect-toolbutton,
>+#inspector-tools > toolbarbutton {
>+  -moz-appearance: none;
>+  width: 78px;

I think you want min-width here.

>+  margin: 3px 5px !important;

Why !important?

>+#inspector-inspect-toolbutton[checked],
>+#inspector-tools > toolbarbutton[checked] {
>+  color: hsl(208,100%,60%) !important;
>+  border-color: hsla(210,8%,5%,.6) !important;
>+  background: -moz-linear-gradient(hsla(220,6%,10%,.6), hsla(210,11%,18%,.45) 75%, hsla(210,11%,30%,.4));
>+  box-shadow: 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 0 hsla(210,16%,76%,.15);
>+}
>+
>+#inspector-inspect-toolbutton[checked]:hover,
>+#inspector-tools > toolbarbutton[checked]:hover {
>+  background-color: transparent!important;
>+}
>+
>+#inspector-inspect-toolbutton[checked]:hover:active,
>+#inspector-tools > toolbarbutton[checked]:hover:active {
>+  background-color: hsla(210,8%,5%,.2)!important;
>+}

Always add a space before !important.
Which of the above !important flags are really needed?
Attachment #559097 - Flags: review?(dao) → review-
(Assignee)

Comment 6

6 years ago
(In reply to Dão Gottwald [:dao] from comment #5)
> Comment on attachment 559097 [details] [diff] [review]
> patch v1
> 
> >+#inspector-toolbar {
> >+  -moz-appearance: none;
> >+  height: 32px;
> 
> Why are you setting a height here rather than letting the toolbar be sized
> automatically based on its contents?

I'll fix that (for Mac and Windows too).

> 
> >+#inspector-inspect-toolbutton,
> >+#inspector-tools > toolbarbutton {
> >+  -moz-appearance: none;
> >+  width: 78px;
> 
> I think you want min-width here.

Right.

> >+  margin: 3px 5px !important;
> 
> Why !important?

Because we find this in toolbarbutton.css:
.toolbarbutton-text {
  margin: 0 !important;
  text-align: center;
}

> >+#inspector-inspect-toolbutton[checked],
> >+#inspector-tools > toolbarbutton[checked] {
> >+  color: hsl(208,100%,60%) !important;
> >+  border-color: hsla(210,8%,5%,.6) !important;
> >+  background: -moz-linear-gradient(hsla(220,6%,10%,.6), hsla(210,11%,18%,.45) 75%, hsla(210,11%,30%,.4));
> >+  box-shadow: 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 3px hsla(210,8%,5%,.25) inset, 0 1px 0 hsla(210,16%,76%,.15);
> >+}
> >+
> >+#inspector-inspect-toolbutton[checked]:hover,
> >+#inspector-tools > toolbarbutton[checked]:hover {
> >+  background-color: transparent!important;
> >+}
> >+
> >+#inspector-inspect-toolbutton[checked]:hover:active,
> >+#inspector-tools > toolbarbutton[checked]:hover:active {
> >+  background-color: hsla(210,8%,5%,.2)!important;
> >+}
> 
> Always add a space before !important.

Ok.

> Which of the above !important flags are really needed?

All of them.
OS: Linux → Mac OS X
(Assignee)

Updated

6 years ago
OS: Mac OS X → Linux
(Assignee)

Comment 7

6 years ago
Merging bug 676255 with this bug. Coming patch will include CSS for Linux, Windows and Mac (pinstripe code updated for consistency with Linux and Windows).
OS: Linux → All
Summary: Make the Highlighter Toolbar look like shorlander's design on Linux → Make the Highlighter Toolbar look like shorlander's design
(Assignee)

Comment 8

6 years ago
Created attachment 563097 [details] [diff] [review]
patch v2

In this new patch, I include code for Linux, Windows and Mac. See comment 7.
Attachment #559097 - Attachment is obsolete: true
Attachment #563097 - Flags: review?(dao)

Updated

6 years ago
Duplicate of this bug: 676255
(Assignee)

Updated

6 years ago
Blocks: 691712
(Assignee)

Comment 10

6 years ago
Dão, do you have some time to look at this patch?
(Assignee)

Updated

6 years ago
Duplicate of this bug: 684188
(Assignee)

Updated

6 years ago
Blocks: 663830
review ping!
Comment on attachment 563097 [details] [diff] [review]
patch v2

>--- a/browser/themes/winstripe/browser/browser.css
>+++ b/browser/themes/winstripe/browser/browser.css

>+#inspector-inspect-toolbutton > .toolbarbutton-icon,
>+#inspector-tools > toolbarbutton  > .toolbarbutton-icon {
>+  margin: 0px;

nit: remove "px"
Attachment #563097 - Flags: review?(dao) → review+
(Assignee)

Comment 14

6 years ago
Created attachment 568665 [details] [diff] [review]
patch v2.1

Thanks for the r+ Dao
(Assignee)

Updated

6 years ago
Whiteboard: [minotaur] → [minotaur][land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/359389696002
Whiteboard: [minotaur][land-in-fx-team] → [minotaur][fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/359389696002
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Hardware: x86 → All
Resolution: --- → FIXED
Whiteboard: [minotaur][fixed-in-fx-team] → [minotaur]
Target Milestone: --- → Firefox 10
You need to log in before you can comment on or make changes to this bug.