Closed Bug 691712 Opened 13 years ago Closed 13 years ago

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

Categories

(DevTools :: General, defect)

Product:
DevTools
Component:
General
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
VERIFIED FIXED
Milestone:
Firefox 10

People

(Reporter: paul, Assigned: paul)

References

Details

(Whiteboard: [testday-20111125])

Attachments

(4 files, 10 obsolete files)

Shorlander's proposal
(deleted), image/png
Details
Resize Grippy
(deleted), image/png
Details
screenshot patch v1.3
(deleted), image/png
shorlander
: ui-review+
Details
patch v1.7
(deleted), patch
Details | Diff | Splinter Review
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).
Attached image Shorlander's proposal (deleted) —
Blocks: 672006
Depends on: 683906
Attached patch patch v1 (obsolete) (deleted) — — Splinter Review
Rob, would that work for you?
Attachment #564507 - Flags: review?(rcampbell)
Attached image screenshot patch v1 (obsolete) (deleted) —
Attached image screenshot patch v1 - with active regions (obsolete) (deleted) —
Attachment #564508 - Flags: ui-review?(shorlander)
Attached patch patch v1.1 (obsolete) (deleted) — — Splinter Review
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]
Attached image Resize Grippy (deleted) —
Can we use this image instead? Or alter the gradient to match it? Mockup: http://cl.ly/1D3A0D0y2a1z3r0U1P11
Ok, I'll update the gradient
Whiteboard: [land-in-fx-team]
Attached patch patch v1.2 (obsolete) (deleted) — — Splinter Review
prettier.
Attachment #564568 - Attachment is obsolete: true
Attached image screenshot patch v1.2 (obsolete) (deleted) —
Attachment #564508 - Attachment is obsolete: true
Attachment #564509 - Attachment is obsolete: true
Attachment #564508 - Flags: ui-review?(shorlander)
Oops, I think this is 2 pixels too small.
Attached patch patch v1.3 (obsolete) (deleted) — — Splinter Review
Attachment #565176 - Attachment is obsolete: true
Attached image screenshot patch v1.3 (deleted) —
Attachment #565177 - Attachment is obsolete: true
Attachment #565181 - Flags: ui-review?(shorlander)
Attachment #565181 - Flags: ui-review?(shorlander) → ui-review+
Attached patch patch v1.4 (obsolete) (deleted) — — Splinter Review
One pixel row was missing
Attachment #565180 - Attachment is obsolete: true
Attachment #565214 - Flags: review?(dao)
Blocks: 663830
Review ping! We have some patches reliant on this.
Status: NEW → ASSIGNED
Attached patch patch v1.5 (obsolete) (deleted) — — Splinter Review
rebased
Attachment #565214 - Attachment is obsolete: true
Attachment #565214 - Flags: review?(dao)
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-
Attached patch patch v1.6 (obsolete) (deleted) — — Splinter Review
Addressed Dao's comments. !important wasn't useful. I was too aggressive with the background:none!important.
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+
Attached patch patch v1.7 (deleted) — — Splinter Review
Thanks for the r+ Dao.
Attachment #569713 - Attachment is obsolete: true
Attachment #568630 - Attachment is obsolete: true
Whiteboard: [land-in-fx-team]
https://hg.mozilla.org/integration/fx-team/rev/ca0e85e9fe89
Whiteboard: [land-in-fx-team] → [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/ca0e85e9fe89
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 10
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]
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: