Closed Bug 1100463 Opened 5 years ago Closed 5 years ago

DevTools Themes Toolbar: Inspector toolbar button has 1px offset when active

Categories

(DevTools :: Framework, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 36

People

(Reporter: aljullu, Assigned: aljullu, Mentored)

Details

Attachments

(2 files)

Attached image Inspectoroffset.png
I think after moving the 'Pick an element from the page' button to the left, the 'Inspector' label has 1px offset when selected. That is only visible when selecting/unselecting the tab.
Attached patch bug1100463.patchSplinter Review
And this patch should solve that.
Attachment #8523986 - Flags: review?(bgrinstead)
Comment on attachment 8523986 [details] [diff] [review]
bug1100463.patch

Review of attachment 8523986 [details] [diff] [review]:
-----------------------------------------------------------------

Good catch.  Do you happen to remember the reason we had :not(:first-child) there in the first place?

Nit: In the future, can you make your commit messages about what the patch is changing instead of stating the problem?  So more like "Get rid of 1px offset on the active Inspector tab button" instead of "Inspector tab button has 1px offset when active".
ni? for comment 2
Flags: needinfo?(aljullu)
(In reply to Brian Grinstead [:bgrins] from comment #2)
> Comment on attachment 8523986 [details] [diff] [review]
> bug1100463.patch
> 
> Review of attachment 8523986 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Good catch.  Do you happen to remember the reason we had :not(:first-child)
> there in the first place?
> 

It was there because 'Inspector' tab was different from the other tabs: it didn't have left border and then, we didn't need to hide it (border-width: 0;) nor to add padding to compensate the width of the hidden border (-moz-padding-start: 1px;).

Now, however, since 'Inspector' tab do have left border when inactive, we need to hide it and set 1px padding like all the other tabs.

>
> Nit: In the future, can you make your commit messages about what the patch
> is changing instead of stating the problem?  So more like "Get rid of 1px
> offset on the active Inspector tab button" instead of "Inspector tab button
> has 1px offset when active".

Sure!
Flags: needinfo?(aljullu)
Attachment #8523986 - Flags: review?(bgrinstead) → review+
https://hg.mozilla.org/mozilla-central/rev/7553b7c1f35f
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 36
[bugday-20150121]

Name: Firefox
Version: 36.0
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:36.0) Gecko/20100101 Firefox/36.0
Multiprocess Windows: 0/1
Channel: Beta release

Status: Fixed-> Verified.

No change in colour ,in case of inactive, of Inspector.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.