Icons for Error Console buttons

RESOLVED DUPLICATE of bug 345477

Status

Toolkit Graveyard
Error Console
--
enhancement
RESOLVED DUPLICATE of bug 345477
12 years ago
10 months ago

People

(Reporter: u88484, Assigned: Kevin Gerich)

Tracking

({fixed1.8, polish})

Trunk
x86
Windows XP
fixed1.8, polish

Details

Attachments

(5 attachments, 14 obsolete attachments)

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
(Reporter)

Description

12 years ago
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.

Comment 1

12 years ago
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?

Updated

12 years ago
Flags: blocking1.8b4? → blocking1.8b4-
(Reporter)

Updated

12 years ago
Depends on: 275265
Flags: blocking1.9a1?
(Reporter)

Updated

12 years ago
Flags: blocking-aviary2.0?
(Reporter)

Comment 2

12 years ago
Created attachment 193454 [details]
icons

the icons

Updated

12 years ago
Keywords: polish
(Reporter)

Comment 3

12 years ago
Created attachment 193457 [details]
console.css

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/
(Reporter)

Comment 4

12 years ago
Created attachment 193471 [details]
screenshot with the changes
(Reporter)

Comment 5

12 years ago
Created attachment 193481 [details] [diff] [review]
winstripe patch
(Reporter)

Updated

12 years ago
Attachment #193481 - Attachment description: patch → winstripe patch
(Reporter)

Comment 6

12 years ago
Created attachment 193486 [details] [diff] [review]
pinstripe patch
(Reporter)

Updated

12 years ago
Attachment #193481 - Flags: review?(benjamin)
(Reporter)

Updated

12 years ago
Attachment #193486 - Flags: review?(benjamin)
(Reporter)

Updated

12 years ago
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)
(Reporter)

Comment 8

12 years ago
Created attachment 193562 [details] [diff] [review]
winstripe patch v1.1

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)
(Reporter)

Comment 9

12 years ago
Created attachment 193563 [details]
updated screenshot including the background colors
Attachment #193471 - Attachment is obsolete: true
(Reporter)

Updated

12 years ago
Attachment #193481 - Flags: review?(kevin)
(Reporter)

Comment 10

12 years ago
Created attachment 193564 [details] [diff] [review]
winstripe patch 1.2

addresses Benjamin Smedberg's comment
Attachment #193562 - Attachment is obsolete: true
Attachment #193564 - Flags: review?(kevin)
(Reporter)

Updated

12 years ago
Attachment #193562 - Flags: review?(kevin)
(Reporter)

Comment 11

12 years ago
Created attachment 193565 [details] [diff] [review]
pinstipe patch v1.1

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)
(Reporter)

Updated

12 years ago
Attachment #193486 - Flags: review?(kevin)
Assignee: bugs → supernova_00
OS: Windows XP → All
Hardware: PC → All
(Assignee)

Comment 12

12 years ago
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-
(Assignee)

Updated

12 years ago
Attachment #193565 - Flags: review?(kevin) → review-
(Reporter)

Comment 13

12 years ago
reassigned to kevin@kmgerich.com.

no problem 
Assignee: supernova_00 → kevin
OS: All → Windows XP
Hardware: All → PC
(Reporter)

Comment 14

12 years ago
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.
(Assignee)

Comment 15

12 years ago
Created attachment 198152 [details] [diff] [review]
new js console styles
Attachment #193564 - Attachment is obsolete: true
Attachment #193565 - Attachment is obsolete: true
Attachment #198152 - Flags: review?(mconnor)
(Assignee)

Comment 16

12 years ago
Created attachment 198153 [details]
Here's a screenshot of the new graphics

Comment 17

12 years ago
(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?
(Assignee)

Comment 18

12 years ago
(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.
(Assignee)

Comment 19

12 years ago
Created attachment 198565 [details]
Alternate JS Console design, with color coded rows

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.
(Reporter)

Comment 20

12 years ago
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.

Comment 21

12 years ago
My 2 cents: I prefer the first screenshot presented by Kevin. Different
background colors (rainbow) make it more difficult to get a good overview. 

Comment 22

12 years ago
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.

Comment 23

12 years ago
Created attachment 198580 [details]
Mockup for the 'All' icon

Just an idea of what the All icon could look like...
(Assignee)

Updated

12 years ago
Attachment #198152 - Flags: review?(mconnor)
(Assignee)

Comment 24

12 years ago
Created attachment 200040 [details] [diff] [review]
JS Console styles, second draft

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)
(Assignee)

Comment 25

12 years ago
Created attachment 200041 [details]
JS Console styles, second draft screenshot
(Assignee)

Comment 26

12 years ago
Created attachment 200042 [details]
console toolbar and bullet icons
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.

Comment 30

12 years ago
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 ;)
(Assignee)

Comment 32

12 years ago
Created attachment 200160 [details] [diff] [review]
round 3 of js console changes incorporating feedback

final changes probably
Attachment #200160 - Flags: review?(mconnor)
(Assignee)

Comment 33

12 years ago
Created attachment 200161 [details]
JS console styles screenshot, round 3
Attachment #200040 - Attachment is obsolete: true
Attachment #200041 - Attachment is obsolete: true
Attachment #200042 - Attachment is obsolete: true

Updated

12 years ago
Attachment #200160 - Flags: review?(mconnor) → review+
(Assignee)

Comment 34

12 years ago
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 35

12 years ago
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+
(Assignee)

Comment 36

12 years ago
Checked in to 1.8 branch. Thanks for the input everyone
Keywords: fixed1.8

Comment 37

12 years ago
Created attachment 200857 [details]
Screenshot of implementation

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?

Comment 38

12 years ago
This doesn't appear to be in trunk builds; is it going to be checked in?
(Reporter)

Updated

11 years ago
Summary: Icons for javascript console buttons → Icons for Error Console buttons

Updated

11 years ago
Depends on: 328065

Comment 39

11 years ago
(In reply to comment #38)
> This doesn't appear to be in trunk builds; is it going to be checked in?

bump
(Reporter)

Updated

11 years ago
Whiteboard: [checkin needed]
Kevin, can you check in whatever is needed on the trunk?

Updated

11 years ago
Blocks: 328065
No longer depends on: 328065

Comment 41

11 years ago
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
Last Resolved: 11 years ago
No longer depends on: 275265
Resolution: --- → DUPLICATE
Whiteboard: [checkin needed]

Updated

10 years ago
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.