Bug 152997 – Web Inspector: Timeline recording controls should use NavigationBar

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Matt Baker]

* SUMMARY
Timeline recording controls should use NavigationBar. Currently the controls are added manually to the DOM. Using NavigationBar will also provide better tooltips for the 'Record' button.

[Matt Baker]

Created attachment 268738 [details]
[Patch] Proposed Fix

[Blaze Burg]

Comment on attachment 268738 [details]
[Patch] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=268738&action=review

Overall it looks like a solid improvement. I have to mark this patch r- because it reverts the fast-toggle behavior of the recording button.

> Source/WebInspectorUI/ChangeLog:7
> +

Please insert the bugzilla description here to describe the rationale for the change.

> Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.css:44
> +    justify-content: center;

Is this change intentional? Does it make it render differently? Can you post a screenshot?

> Source/WebInspectorUI/UserInterface/Views/TimelineSidebarPanel.js:872
>      {

Like for breakpoints, we used to preemptively change the recording button to look 'on' before hearing from the backend whether the command was successful. This was to reduce apparent latency when remote inspecting and on slow/loaded machines. Even in the local case, RTT can be 100ms to start a recording, causing the UI to appear laggy. Can you revert this change?

(/cc Tim, who fixed this the last time I regressed it)

[Matt Baker]

Created attachment 268820 [details]
[Image] Screenshot showing justify-content on/off

[Matt Baker]

Created attachment 268823 [details]
[Patch] Proposed Fix

[Blaze Burg]

Comment on attachment 268823 [details]
[Patch] Proposed Fix

r=me

I'm not super keen about the new text alignment, but I am not super attached to the current navigation bar placement. It might work better once the sidebar is removed.

[WebKit Commit Bot]

Comment on attachment 268823 [details]
[Patch] Proposed Fix

Clearing flags on attachment: 268823

Committed r194936: <http://trac.webkit.org/changeset/194936>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/24193458>