Closed Bug 843287 Opened 7 years ago Closed 7 years ago

UI improvements for the variables view and sidebar

Categories

(DevTools :: Console, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 23

People

(Reporter: msucan, Assigned: vporof)

References

Details

Attachments

(2 files, 2 obsolete files)

In bug 808370 comment #32 and #33 Panos and Victor made suggestions for visual improvements.

Panos said:
> The larger font remains, and my guess is that the cause is the slightly
> larger input box (the old one is constrained by the toolbar height,
> apparently). There are a few other visual nits I've observed, like the width
> of the splitter which is larger than the ones we use in the debugger, or the
> 1 px border of the scope header that leaks into the console output area.

Victor said:
> No, it's because of type="search". Don't ask, I know it's delightfully
> unexpected.
> A simple font-size: inherit does the trick, I'm taking care of it in a
> different bug.
> 
> A few more oddities I noticed (most of these can be fixed in followups, but
> please file bugs):
> * the variables view "leaks" UI elements inside the webconsole output window
> (see [object Proxy] header in the screenshot)
> * the expander on a scope header (arrow/twisty) should be hidden, since it
> can't be collapsed
> * there's a gray border around the variables view container (especially
> noticeable on the top
> * the side margin is 1 pixel too thick (probably related to the gray border)
> * there's one weird black pixel on the far right of the toolbar (immediately
> above the clear button)
> * I noticed sometimes the filtering doesn't work on the first try; I don't
> understand why this is happening in the webconsole and not in the debugger,
> we need to investigate!
> * the variables view from a console.dir output should probably have a 1px
> light gray border; the sizing and margins are perfect, thanks for the update!
> * the debugger uses accel+shift+v to focus the variables view; maybe the
> webconsole should do the same?

Paul: do you have any suggestions?
Attached patch v1 (obsolete) — Splinter Review
Attachment #735239 - Flags: review?(mihai.sucan)
Comment on attachment 735239 [details] [diff] [review]
v1

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

::: browser/themes/linux/devtools/webconsole.css
@@ -245,5 @@
>    margin-bottom: 15px;
>  }
>  
> -.devtools-side-splitter {
> -  background: #666;

Just asking: do you really want to change the splitter color here? I think all splitters should have the same color. PS -moz-border-start: #666 is your friend, background is your enemy :)
Attached patch v2 (obsolete) — Splinter Review
Apparently using display:none on the sidebar tabs made Sidebar.jsm get all cranky on a webconsole test. OK FINE, you win this time you pesky sidebar!

I switched to height: 0 as it was previously but added border: none to hide that annoying upper border. The left gray border was caused by the splitter.
Assignee: nobody → vporof
Attachment #735239 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #735239 - Flags: review?(mihai.sucan)
Attachment #735342 - Flags: review?(mihai.sucan)
Attached patch v2Splinter Review
Wrong patch, apparently I'm tired.
Attachment #735342 - Attachment is obsolete: true
Attachment #735342 - Flags: review?(mihai.sucan)
Attachment #735343 - Flags: review?(mihai.sucan)
Duplicate of this bug: 860485
Comment on attachment 735343 [details] [diff] [review]
v2

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

UI looks better now. Thank you!

Two questions:
1. Why did you remove all of the splitter stuff from webconsole.js?
2. Should you add a border around the vview for console.dir() instances? As was suggested, see comment #0.
Attachment #735343 - Flags: review?(mihai.sucan) → review+
(In reply to Mihai Sucan [:msucan] from comment #7)
> Comment on attachment 735343 [details] [diff] [review]
> v2
> 
> Review of attachment 735343 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> UI looks better now. Thank you!
> 
> Two questions:
> 1. Why did you remove all of the splitter stuff from webconsole.js?

It was useless, since the "state" attribute doesn't do anything yet (the splitter doesn't collapse the sidebar at this point). And I think we need a button for such things, because clickable splitters are ugly and annoying.

> 2. Should you add a border around the vview for console.dir() instances? As
> was suggested, see comment #0.

Good idea.
Attached patch v2.1Splinter Review
Attachment #736380 - Flags: review+
Priority: -- → P2
https://hg.mozilla.org/mozilla-central/rev/53eb20bcce0d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 23
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.