Last Comment Bug 764746 - [devtb] style the error counter
: [devtb] style the error counter
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Developer Tools (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Firefox 16
Assigned To: Paul Rouget [:paul]
:
Mentors:
Depends on: 762996 764555 777569 781291
Blocks:
  Show dependency treegraph
 
Reported: 2012-06-14 02:46 PDT by Paul Rouget [:paul]
Modified: 2012-08-08 11:39 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
screenshot work in progress (920.79 KB, image/png)
2012-06-25 06:09 PDT, Paul Rouget [:paul]
no flags Details
v1 (6.13 KB, patch)
2012-06-25 06:37 PDT, Paul Rouget [:paul]
no flags Details | Diff | Review
v1.1 (6.15 KB, patch)
2012-06-25 06:42 PDT, Paul Rouget [:paul]
no flags Details | Diff | Review
screenshot: what if we make the text red too? (41.42 KB, image/png)
2012-06-25 06:43 PDT, Paul Rouget [:paul]
no flags Details
v1.2 (5.94 KB, patch)
2012-06-26 08:23 PDT, Paul Rouget [:paul]
mihai.sucan: review+
Details | Diff | Review
v1.3 (7.74 KB, patch)
2012-06-26 15:15 PDT, Paul Rouget [:paul]
no flags Details | Diff | Review
v1.4 (5.09 KB, patch)
2012-06-27 07:13 PDT, Paul Rouget [:paul]
dao+bmo: review+
Details | Diff | Review
v1.5 (5.21 KB, patch)
2012-07-05 08:03 PDT, Paul Rouget [:paul]
no flags Details | Diff | Review

Description Paul Rouget [:paul] 2012-06-14 02:46:17 PDT

    
Comment 1 Paul Rouget [:paul] 2012-06-25 06:09:59 PDT
Created attachment 636290 [details]
screenshot work in progress

Basic approach: we replace the icon with the counter.
Comment 2 Paul Rouget [:paul] 2012-06-25 06:37:58 PDT
Created attachment 636292 [details] [diff] [review]
v1
Comment 3 Paul Rouget [:paul] 2012-06-25 06:42:33 PDT
Created attachment 636293 [details] [diff] [review]
v1.1
Comment 4 Paul Rouget [:paul] 2012-06-25 06:43:33 PDT
Created attachment 636294 [details]
screenshot: what if we make the text red too?
Comment 5 Paul Rouget [:paul] 2012-06-25 15:56:14 PDT
Comment on attachment 636293 [details] [diff] [review]
v1.1

(wrong patch, sorry)
Comment 6 Paul Rouget [:paul] 2012-06-26 08:23:27 PDT
Created attachment 636719 [details] [diff] [review]
v1.2
Comment 7 Paul Rouget [:paul] 2012-06-26 08:24:23 PDT
Comment on attachment 636719 [details] [diff] [review]
v1.2

Can you please review the devtools code?
Comment 8 Paul Rouget [:paul] 2012-06-26 08:24:51 PDT
Comment on attachment 636719 [details] [diff] [review]
v1.2

I need a browser review too!
Comment 9 Paul Rouget [:paul] 2012-06-26 11:13:12 PDT
For some reasons, I forgot to add "-moz-box-pack:center" to the pseudo element.
I will add that after the reviews.
Comment 10 Mihai Sucan [:msucan] 2012-06-26 12:25:47 PDT
Comment on attachment 636719 [details] [diff] [review]
v1.2

Patch looks fine and UI is awesome. I like the toolbar changes. It looks much better.

The :( smiley doesn't show. I get:

Error: NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIStringBundle.GetStringFromName]
Source File: resource:///modules/devtools/DeveloperToolbar.jsm
Line: 500


Giving the patch r+ but please fix this.
Comment 11 Paul Rouget [:paul] 2012-06-26 15:15:54 PDT
Created attachment 636897 [details] [diff] [review]
v1.3

mssing entry in jar.mn
Comment 12 Dão Gottwald [:dao] 2012-06-27 06:41:40 PDT
Comment on attachment 636897 [details] [diff] [review]
v1.3

>+# The error counter is located in front of the Web Console button in the developer
>+# toolbar. This counter can only hold 2 characters.

Why can it only hold 2 characters?
Comment 13 Paul Rouget [:paul] 2012-06-27 06:55:03 PDT
(In reply to Dão Gottwald [:dao] from comment #12)
> Comment on attachment 636897 [details] [diff] [review]
> v1.3
> 
> >+# The error counter is located in front of the Web Console button in the developer
> >+# toolbar. This counter can only hold 2 characters.
> 
> Why can it only hold 2 characters?

Technically, it can. But I don't want the counter to grow and move all the buttons when it reaches 100. I want the counter to look like it replaces the icon.
Comment 14 Dão Gottwald [:dao] 2012-06-27 07:03:28 PDT
":(" is a nice gimmick, but not very informative or self-explanatory. Between this and the counter growing slightly when reaching 100, I think the latter is preferable. "99+" would be ok too, but of course this has the same problem that you tried to avoid.
Comment 15 Paul Rouget [:paul] 2012-06-27 07:13:36 PDT
Created attachment 637107 [details] [diff] [review]
v1.4
Comment 16 Paul Rouget [:paul] 2012-07-03 02:58:04 PDT
review ping
Comment 17 Dão Gottwald [:dao] 2012-07-05 07:44:59 PDT
Comment on attachment 637107 [details] [diff] [review]
v1.4

>+#developer-toolbar-webconsole[error-count]:before {
>+  content: attr(error-count);
>+  display: -moz-box;
>+  -moz-box-pack: center;

This part should probably be in a content stylesheet.
Comment 18 Paul Rouget [:paul] 2012-07-05 08:03:09 PDT
Created attachment 639345 [details] [diff] [review]
v1.5

thank you Dao.
Comment 20 Tim Taubert [:ttaubert] 2012-07-06 04:01:23 PDT
https://hg.mozilla.org/mozilla-central/rev/7c1d395ebab7

Note You need to log in before you can comment on or make changes to this bug.