Add some pretty resizers to the Highlighter toolbar when the HTML tree panel is open

VERIFIED FIXED in Firefox 10

Status

()

Firefox
Developer Tools
VERIFIED FIXED
6 years ago
6 years ago

People

(Reporter: paul, Assigned: paul)

Tracking

Trunk
Firefox 10
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [testday-20111125])

Attachments

(4 attachments, 10 obsolete attachments)

(Assignee)

Description

6 years ago
We want to be able to resize the tree panel. The current situation (using the free space in the toolbar as a resizer) is blocking further developments in the toolbar (like the breadcrumbs).
(Assignee)

Comment 1

6 years ago
Created attachment 564506 [details]
Shorlander's proposal
(Assignee)

Updated

6 years ago
Blocks: 672006
Depends on: 683906
(Assignee)

Comment 2

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

Rob, would that work for you?
Attachment #564507 - Flags: review?(rcampbell)
(Assignee)

Comment 3

6 years ago
Created attachment 564508 [details]
screenshot patch v1
(Assignee)

Comment 4

6 years ago
Created attachment 564509 [details]
screenshot patch v1 - with active regions
(Assignee)

Updated

6 years ago
Attachment #564508 - Flags: ui-review?(shorlander)
(Assignee)

Updated

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

Comment 6

6 years ago
Created attachment 564568 [details] [diff] [review]
patch v1.1

CSS tweaks.
Attachment #564507 - Attachment is obsolete: true
Attachment #564507 - Flags: review?(rcampbell)
Attachment #564568 - Flags: review?(rcampbell)
Comment on attachment 564568 [details] [diff] [review]
patch v1.1

fantastique!
Attachment #564568 - Flags: review?(rcampbell) → review+
Whiteboard: [land-in-fx-team]
Created attachment 565030 [details]
Resize Grippy

Can we use this image instead? Or alter the gradient to match it?

Mockup: http://cl.ly/1D3A0D0y2a1z3r0U1P11
(Assignee)

Comment 9

6 years ago
Ok, I'll update the gradient
Whiteboard: [land-in-fx-team]
(Assignee)

Comment 10

6 years ago
Created attachment 565176 [details] [diff] [review]
patch v1.2

prettier.
Attachment #564568 - Attachment is obsolete: true
(Assignee)

Comment 11

6 years ago
Created attachment 565177 [details]
screenshot patch v1.2
Attachment #564508 - Attachment is obsolete: true
Attachment #564509 - Attachment is obsolete: true
Attachment #564508 - Flags: ui-review?(shorlander)
(Assignee)

Comment 12

6 years ago
Oops, I think this is 2 pixels too small.
(Assignee)

Comment 13

6 years ago
Created attachment 565180 [details] [diff] [review]
patch v1.3
Attachment #565176 - Attachment is obsolete: true
(Assignee)

Comment 14

6 years ago
Created attachment 565181 [details]
screenshot patch v1.3
Attachment #565177 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #565181 - Flags: ui-review?(shorlander)
Attachment #565181 - Flags: ui-review?(shorlander) → ui-review+
(Assignee)

Comment 15

6 years ago
Created attachment 565214 [details] [diff] [review]
patch v1.4

One pixel row was missing
Attachment #565180 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #565214 - Flags: review?(dao)
(Assignee)

Updated

6 years ago
Blocks: 663830
Review ping! We have some patches reliant on this.
Status: NEW → ASSIGNED
(Assignee)

Comment 17

6 years ago
Created attachment 568630 [details] [diff] [review]
patch v1.5

rebased
(Assignee)

Updated

6 years ago
Attachment #565214 - Attachment is obsolete: true
Attachment #565214 - Flags: review?(dao)
(Assignee)

Updated

6 years ago
Attachment #568630 - Flags: review?(dao)
Comment on attachment 568630 [details] [diff] [review]
patch v1.5

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

> #inspector-toolbar {
>   -moz-appearance: none;
>-  padding: 4px 3px;
>+  padding: 0px 3px 4px 3px;

nit: 0 instead of 0px

>+#inspector-end-resizer {
>+  width: 12px; height: 8px;

new line after ;

>+  background-image: -moz-linear-gradient(top,black 1px,rgba(255, 255, 255, 0.2) 1px) !important;

Why do you need !important here?

nit: add a space after each comma, *except* within color functions:
-moz-linear-gradient(top, black 1px, rgba(255,255,255,0.2) 1px)

>@@ -2670,8 +2688,9 @@ panel[dimmed="true"] {
> #highlighter-nodeinfobar-container[position="top"] > #highlighter-nodeinfobar,
> #highlighter-nodeinfobar-container[position="overlap"] > #highlighter-nodeinfobar {
>   box-shadow: 0 1px 0 hsla(0, 0%, 100%, .1) inset;
> }
> 
> #highlighter-nodeinfobar-container[hide-arrow] > #highlighter-nodeinfobar {
>   margin: 7px 0;
> }
>+

bogus change
so, r+ with those changes, Dao?
Comment on attachment 568630 [details] [diff] [review]
patch v1.5

I'd prefer seeing a new patch, as I've seen people mess up trivial changes, forget some of our three themes, etc.

Also waiting for a response regarding !important.

By the way, write "padding: 0 3px 4px;" instead of "padding: 0 3px 4px 3px;". This makes it more obvious that the padding is symmetrical (so RTL won't be an issue).
Attachment #568630 - Flags: review?(dao) → review-
(Assignee)

Comment 21

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

Addressed Dao's comments. !important wasn't useful. I was too aggressive with the background:none!important.
(Assignee)

Updated

6 years ago
Attachment #569713 - Flags: review?(dao)
Comment on attachment 569713 [details] [diff] [review]
patch v1.6

>+          <toolbarseparator />

nit: remove the space before />

>+    let resizerTop = 

trailing space

>+    let resizerEnd = 

ditto

>+#inspector-end-resizer {

>+  border-width: 1px 1px 0 1px;

border-width: 1px 1px 0;
(affects all themes)

>+  margin: 7px 7px 8px 7px;

margin: 7px 7px 8px;
(affects all themes)
Attachment #569713 - Flags: review?(dao) → review+
(Assignee)

Comment 23

6 years ago
Created attachment 569827 [details] [diff] [review]
patch v1.7

Thanks for the r+ Dao.
(Assignee)

Updated

6 years ago
Attachment #569713 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Attachment #568630 - Attachment is obsolete: true
(Assignee)

Updated

6 years ago
Whiteboard: [land-in-fx-team]

Comment 24

6 years ago
https://hg.mozilla.org/integration/fx-team/rev/ca0e85e9fe89
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]

Comment 25

6 years ago
https://hg.mozilla.org/mozilla-central/rev/ca0e85e9fe89
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 10

Comment 26

6 years ago
Mozilla/5.0 (Windows NT 5.1; rv:10.0a2) Gecko/20111123 Firefox/10.0a2
Verified fixed. http://screencast.com/t/0IGuh6gZpy1s
Status: RESOLVED → VERIFIED
Whiteboard: [testday-20111125]
You need to log in before you can comment on or make changes to this bug.