Closed Bug 291002 Opened 16 years ago Closed 15 years ago

Icons for Error Console buttons

Categories

(Toolkit Graveyard :: Error Console, enhancement)

x86
Windows XP
enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 345477

People

(Reporter: u88484, Assigned: kevin)

References

Details

(Keywords: fixed1.8, polish)

Attachments

(5 files, 14 obsolete files)

6.85 KB, image/png
Details
2.23 KB, image/png
Details
5.43 KB, patch
mconnor
: review+
Details | Diff | Splinter Review
36.90 KB, image/jpeg
Details
25.38 KB, image/gif
Details
Could we use the TB icons for the javascript console buttons? They would fit in
with the FF theme and they make the buttons easier to click plus makes the
console not so plain.
Would it be possible to fix this as a polish bug maybe even after 1.5 beta?  And
maybe get the background color behind each entry (Message/Warning/Error) like on
the Mac.
Flags: blocking1.8b4?
Flags: blocking1.8b4? → blocking1.8b4-
Depends on: 275265
Flags: blocking1.9a1?
Flags: blocking-aviary2.0?
Attached image icons
the icons
Keywords: polish
Attached file console.css (obsolete) —
modified console.css

new code for using the icons is att he very bottom, needs clean up a little
since i'm not an css expert. Also I have no clue how to make diff files so if
someone can explain I will be happy to do that and attach a patch.

both of these files need to go in classic.jar in skin/classic/global/console/
Attached image screenshot with the changes (obsolete) —
Attached patch winstripe patch (obsolete) — Splinter Review
Attachment #193481 - Attachment description: patch → winstripe patch
Attached patch pinstripe patch (obsolete) — Splinter Review
Attachment #193481 - Flags: review?(benjamin)
Attachment #193486 - Flags: review?(benjamin)
Attachment #193457 - Attachment is obsolete: true
Attachment #193481 - Flags: review?(benjamin) → review?(kevin)
Comment on attachment 193486 [details] [diff] [review]
pinstripe patch

Please fix the "no newline at end of file"
Attachment #193486 - Flags: review?(benjamin) → review?(kevin)
Attached patch winstripe patch v1.1 (obsolete) — Splinter Review
this patch adds background color behind each entry (Message/Warning/Error) like
on
the Mac.
Attachment #193481 - Attachment is obsolete: true
Attachment #193562 - Flags: review?(kevin)
Attachment #193471 - Attachment is obsolete: true
Attachment #193481 - Flags: review?(kevin)
Attached patch winstripe patch 1.2 (obsolete) — Splinter Review
addresses Benjamin Smedberg's comment
Attachment #193562 - Attachment is obsolete: true
Attachment #193564 - Flags: review?(kevin)
Attachment #193562 - Flags: review?(kevin)
Attached patch pinstipe patch v1.1 (obsolete) — Splinter Review
addresses Benjamin Smedberg's comment

sorry about all these edits...first time submitting patches.
Attachment #193486 - Attachment is obsolete: true
Attachment #193565 - Flags: review?(kevin)
Attachment #193486 - Flags: review?(kevin)
Assignee: bugs → supernova_00
OS: Windows XP → All
Hardware: PC → All
Comment on attachment 193564 [details] [diff] [review]
winstripe patch 1.2

Kurt,
Thanks for your work on this. I wish I'd seen this bug sooner though. I'm
planning to do a makeover on the Winstripe JS console that hopefully will get
in for 1.5, including icons with the Winstripe style. Can you please reassign
to me? I'll use your winstripe patch as a starting point.
Attachment #193564 - Flags: review?(kevin) → review-
Attachment #193565 - Flags: review?(kevin) → review-
reassigned to kevin@kmgerich.com.

no problem 
Assignee: supernova_00 → kevin
OS: All → Windows XP
Hardware: All → PC
Kevin how is this looking for 1.5? If looks like it won't make 1.5, can we at
least use my patches to make the console at least a little better looking and
easier to view because of the background colors for different errors. No risk
patch and no l10n affect so can be landed after 1.8b5.
Attached patch new js console styles (obsolete) — Splinter Review
Attachment #193564 - Attachment is obsolete: true
Attachment #193565 - Attachment is obsolete: true
Attachment #198152 - Flags: review?(mconnor)
Attached image Here's a screenshot of the new graphics (obsolete) —
(In reply to comment #16)
> Created an attachment (id=198153) [edit]
> Here's a screenshot of the new graphics

How come the small icons are missing the sign within?
(In reply to comment #17)
> How come the small icons are missing the sign within?

To reduce the visual weight of the bullet icons while stil providing a clue
about the nature of the error. Also I didn't want to use the same icons in the
toolbar and on the side because you don't interact with them the same way.
We could do something like this, which eliminates the row icon altogether. The
downside is to get the color coded backgrounds we would have to hardcode both
background and foreground colors.
Kevin, I think those icons plus the background colors would look great
together...with just icons or just background color, the window seems like it's
missing something. But with both, looks pretty darn good.
My 2 cents: I prefer the first screenshot presented by Kevin. Different
background colors (rainbow) make it more difficult to get a good overview. 
Colors are bad, just look at any remotely similar program, like the event viewer
in Windows and imagine it with color marked lines and not just icons.

Another question, what is the all button supposed to symbolize?
I have tried to figure this one out, but nothing makes sense.
Just an idea of what the All icon could look like...
Attachment #198152 - Flags: review?(mconnor)
Attached patch JS Console styles, second draft (obsolete) — Splinter Review
OK, here is the patch, without the color coded rows and with horizontal icons
Attachment #193563 - Attachment is obsolete: true
Attachment #198152 - Attachment is obsolete: true
Attachment #198153 - Attachment is obsolete: true
Attachment #198565 - Attachment is obsolete: true
Attachment #200040 - Flags: review?(mconnor)
Attached file console toolbar and bullet icons (obsolete) —
Comment on attachment 200040 [details] [diff] [review]
JS Console styles, second draft

Beltzner has some suggested tweaks if you have more cycles, but this is good to
go.
Attachment #200040 - Flags: review?(mconnor) → review+
*** Bug 266539 has been marked as a duplicate of this bug. ***
(In reply to comment #27)
> (From update of attachment 200040 [details] [diff] [review] [edit])
> Beltzner has some suggested tweaks if you have more cycles, but this is good to
> go.
> 

I've opened bug 312962 to track those suggestions since we're coming up fast on
code freeze. Some of the issues identified in that bug might be able to be done
here, though:

  - right align the source file and line number:
  - remove labels "Error", "Warning", "Message", "Souce File"

............................................................................
  ***  foo is not defined
 ***** 
  ***                               _javascript: document.all_    Line    1    
                        
''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''''

I also suggest making the icons in the content area smaller versions of the ones
that appear in the toolbar, but that requires new graphics so might not be
do-able for 1.5.
Is right aligning the source really a good idea?  The case that I think  could
be a problem is when you get a very long source URL.  Then other shorter source
URLs could end up not being seen because you have to scroll horizontaly for them
to be visible.  

From my JS console (mangled a bit)
http://us.f519.mail.yahoo.com/ym/ShowLetter?MsgId=----_5579687_54466_1534_624_0_8962_1200_1288318393&Idx=0&YY=56931&inc=25&order=down&sort=date&pos=0&view=&head=&box=Inbox
vs
example given
Javascript:document.all

So perhaps any extra changes besides removing the labels should wait.  
Hm. I'd originally been thinking that we'd use a right-align for the file name,
then some padding and then a left-align for the number, so:

[..........................................file.name]  Line: [number]

But to support that I guess we'd need code that truncates long labels (like in
Windows Explorer or OS X Finder, so you'd get "reallyLongF...ame" and have a
hover-help tip that contains the full filename) which would be more work than we
have time for. Unless anyone can think of a way around this, then yeah, let's
just strip the labels and keep current alignment for now.

I'll be pouting in this corner over here ;)
final changes probably
Attachment #200160 - Flags: review?(mconnor)
Attachment #200040 - Attachment is obsolete: true
Attachment #200041 - Attachment is obsolete: true
Attachment #200042 - Attachment is obsolete: true
Attachment #200160 - Flags: review?(mconnor) → review+
Comment on attachment 200160 [details] [diff] [review]
round 3 of js console changes incorporating feedback

low risk. good polish. requesting approval to land
Attachment #200160 - Flags: approval1.8rc1?
Comment on attachment 200160 [details] [diff] [review]
round 3 of js console changes incorporating feedback

Kevin, we won't have an opportunity to respin if this breaks anything. I'm approving so if you're confident in this, go ahead and land.
Attachment #200160 - Flags: approval1.8rc1? → approval1.8rc1+
Checked in to 1.8 branch. Thanks for the input everyone
Keywords: fixed1.8
The "Line x" text is not equally aligned for all rows.

Also, the errors are not easy to read as before because of the bold text (my 2 cents)
Flags: blocking1.8b5-
Flags: blocking-aviary2?
This doesn't appear to be in trunk builds; is it going to be checked in?
Summary: Icons for javascript console buttons → Icons for Error Console buttons
Depends on: 328065
(In reply to comment #38)
> This doesn't appear to be in trunk builds; is it going to be checked in?

bump
Whiteboard: [checkin needed]
Kevin, can you check in whatever is needed on the trunk?
Blocks: 328065
No longer depends on: 328065
Gavin is there anything needed from this bug on the trunk that won't be covered by bug 345477?  That bug seems to take over the purpose of this bug.  I think the changes checked in from this bug were just to get something done for 1.5 with the hope that something better would be done in the future.  The something better in terms of usability would be covered by Bug 312962.
(In reply to comment #41)
> Gavin is there anything needed from this bug on the trunk that won't be covered
> by bug 345477?

I don't know what is going to be changed in bug 345477, so I can't really say.
Status: NEW → ASSIGNED
Dupe of bug 345477 at this point?
Sure.

*** This bug has been marked as a duplicate of 345477 ***
No longer blocks: 328065
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
No longer depends on: 275265
Resolution: --- → DUPLICATE
Whiteboard: [checkin needed]
Flags: blocking1.9a1?
Product: Firefox → Toolkit
Product: Toolkit → Toolkit Graveyard
You need to log in before you can comment on or make changes to this bug.