Bug 20055

Summary: Line highlighting should last longer
Product: WebKit Reporter: Anthony Ricaud <rik>
Component: Web Inspector (Deprecated)Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben, hyatt, timothy
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch to restore the fade out
timothy: review-
Patch with a class for fade out
none
Increase the highlighted time timothy: review+

Description Anthony Ricaud 2008-07-16 09:32:08 PDT
When a line is highlighted (e.g : after a click in the profile panel), there's not enough time for the user to see it.
Comment 1 Anthony Ricaud 2008-07-16 09:41:17 PDT
Created attachment 22308 [details]
Patch to restore the fade out
Comment 2 Anthony Ricaud 2008-07-16 09:45:31 PDT
In addition to this fix, I also suggest to increase the effect time. Either the time with the className or the transition.
Comment 3 Timothy Hatcher 2008-07-16 11:04:12 PDT
Comment on attachment 22308 [details]
Patch to restore the fade out

Why not move the "-webkit-transition-property: background-color; -webkit-transition-duration: 1s;" style properties to the normal ".webkit-line-content" rule a couple of lines above? That should remove the need to set the inline transition styles.

r- until that is changed, unless there is a reason it can't be that way.
Comment 4 Anthony Ricaud 2008-07-16 11:12:08 PDT
Two reasons:
aroben told me that the desired effect was only a fade out. With the transition on the CSS rule, it would have been a fade in and out.
And even if we want to change for a fade in and out, this rule cause an effect from black to white when the HTML source is created.
Comment 5 Timothy Hatcher 2008-07-16 11:18:15 PDT
(In reply to comment #4)
> Two reasons:
> aroben told me that the desired effect was only a fade out. With the transition
> on the CSS rule, it would have been a fade in and out.
> And even if we want to change for a fade in and out, this rule cause an effect
> from black to white when the HTML source is created.
> 

I see. Then I would recommend adding a new CSS class rule that "turns" on the transitions, and add the class after "webkit-highlighted-line" is added. Then remove both classes after the timeout.
Comment 6 Anthony Ricaud 2008-07-18 06:26:58 PDT
Created attachment 22358 [details]
Patch with a class for fade out

I've added the class. But I'm concerned about classes in this file. It will be difficult for other ports to style the inspector differently.
Comment 7 Timothy Hatcher 2008-07-18 09:27:19 PDT
(In reply to comment #6)
> Created an attachment (id=22358) [edit]
> Patch with a class for fade out
> 
> I've added the class. But I'm concerned about classes in this file. It will be
> difficult for other ports to style the inspector differently.
> 

I agree but it is no better than the inline style.
Comment 8 Anthony Ricaud 2008-07-18 09:37:19 PDT
Of course, inline styles are worst. I was just comparing with an external CSS file.
Comment 9 Mark Rowe (bdash) 2008-07-26 22:10:27 PDT
Landed in r35382.
Comment 10 Anthony Ricaud 2008-07-27 03:09:01 PDT
Reopen this bug because no decision was taken about making this effect longer.
Comment 11 Timothy Hatcher 2008-07-27 16:56:41 PDT
I think a fade of 2s or 3s would be fine.
Comment 12 Timothy Hatcher 2008-07-27 16:57:43 PDT
I mean fade in 2 or 3s.
Comment 13 Mark Rowe (bdash) 2008-07-28 00:39:14 PDT
If the patch that was landed did not address this bug, why was it attached to this bug report?
Comment 14 Mark Rowe (bdash) 2008-07-28 00:39:30 PDT
Comment on attachment 22358 [details]
Patch with a class for fade out

Clearing the review flag as the patch has been landed.
Comment 15 Anthony Ricaud 2008-07-28 00:59:22 PDT
(In reply to comment #12)
> I mean fade in 2 or 3s.
> 

You mean fade out, since there's no fade in.

(In reply to comment #13)
> If the patch that was landed did not address this bug, why was it attached to
> this bug report?
> 
The patch adressed one part of the bug.
Comment 16 Anthony Ricaud 2008-08-05 16:37:37 PDT
Created attachment 22665 [details]
Increase the highlighted time
Comment 17 Timothy Hatcher 2008-08-07 20:58:19 PDT
Fixed in r35633.