Currently, there are no visual indicators when the user is inspecting or not, or when the Style or DOM panels are enabled or not. We should make sure that the buttons on the Inspector toolbar stay pressed when their functionality is enabled.

Proposed patch. The toolbar looks nicer now. :)

Comment on attachment 476359 [details] [diff] [review] proposed patch this is not really the styling I was hoping for with this bug. I'd prefer to find some toolbar and button styling similar to what's in the bookmarks toolbar, but haven't had a chance to go digging for it. Maybe you'd like to

Comment on attachment 476359 [details] [diff] [review] proposed patch Talked with mihai in irc and it sounds like this styles correctly on linux, so I'm betting we can either add in mac and windows styling on top or afterwards. f+

Updated patch. This fixes a typo. Thanks Robert for the feedback+!

Comment on attachment 479781 [details] [diff] [review] updated patch Asking for review from Dolske.

one comment on the unittest. We're checking the checked state on these buttons after the UI is closed. Seems kind of wrong. A better test would be to test the opening and closing of the panels and inspect mode with the inspector UI up.

sorry to follow-up with yet another comment about that test, but another alternative could be to put the individual panel check state tests (whew) into each panel's test. I.e., test the Style panel's check state in the style panel tests. Check the DOM panel tests in the DOM panel's tests, etc. I'll leave which method you choose up to you.

Patch update 3. Thanks Robert for your suggestion - good points. I have included code that toggles each panel on/off (and inspection), to check if their states are properly updated. I included this change in the initialization test, because to me it feels like these things belong there. The panel-specific tests are related to the panel implementations - not the more general aspects of how the Inspector works (open/close of each panel and all that). I hope this is fine.

an attempt at better bookmark styling for the Mac, impacts other platforms or requires updating to toolbar styling on windows and linux. Reusing bookmark toolbar style rules to create "checked" buttons on the toolbar. Seems to blend well with the rest of the UI. Not sure if I should copy and paste those rules into another selector or not. comments welcome.

(In reply to comment #9) > Created attachment 480594 [details] [diff] [review] > Inspector toolbar update, with Mac styling > > an attempt at better bookmark styling for the Mac, impacts other platforms or > requires updating to toolbar styling on windows and linux. > > Reusing bookmark toolbar style rules to create "checked" buttons on the > toolbar. Seems to blend well with the rest of the UI. Not sure if I should copy > and paste those rules into another selector or not. > > comments welcome. On Linux there's some added padding for the inspector buttons. If I copy the new CSS rule from the pinstripe theme over to the gnomestripe theme, the toolbar looks bad. I think on Linux the default was fine. Maybe we can do the style changes only for Mac? Also, the patch does not include the changes I did for update 3 (attachment 480147 [details] [diff] [review]).

I changed this on the mac and windows (though incomplete there), to make it work. I think to do this properly we'll need to create a separate inspector toolbar button class and copy in the bookmark styling there. As it is, I've added the classes to the xul definitions which blew away your changes. We probably need a combination of the two versions to work on all platforms.

(In reply to comment #11) > I changed this on the mac and windows (though incomplete there), to make it > work. I think to do this properly we'll need to create a separate inspector > toolbar button class and copy in the bookmark styling there. As it is, I've > added the classes to the xul definitions which blew away your changes. > > We probably need a combination of the two versions to work on all platforms. Indeed. Shall I combine the two patches then? With the changes you suggested.

go for it!

Patch update 4. I have combined the two patches. Please let me know if it's fine. If yes, I'll replace the one that is currently waiting for review. Thanks! (Please note that I was able to test only on Linux.)

Comment on attachment 480147 [details] [diff] [review] patch update 3 AIUI this code isn't enabled in FF4 any more, so clearing review. Reflag when we start working on post-FF4 things.

This is no longer a valid bug report. Plans have changed.

Comment on attachment 481902 [details] [diff] [review] patch update 4 (mac styling) oh geez. This is old.