Bug 150703 – Web Inspector: Convert TimelineRuler to View base class

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

[Matt Baker]

* SUMMARY
Convert TimelineRuler to View base class. TimelineRuler can update markers and selection separately from the rest of its content, which may require additional layout logic in View.js.

[Radar WebKit Bug Importer]

<rdar://problem/23326721>

[Matt Baker]

Created attachment 264722 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

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

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

> Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:340
> -        var visibleWidth = this._recalculate();
> +        let visibleWidth = this.element.clientWidth;

Why this change?

> Source/WebInspectorUI/UserInterface/Views/View.js:44
> +    get isLayoutPending()

This should be a function if it is prefixed with "is".

[Matt Baker]

(In reply to comment #3)
> Comment on attachment 264722 [details]
> [Patch] Proposed Fix
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=264722&action=review
> 
> > Source/WebInspectorUI/UserInterface/Views/TimelineRuler.js:340
> > -        var visibleWidth = this._recalculate();
> > +        let visibleWidth = this.element.clientWidth;
> 
> Why this change?

I think this was an error.

> > Source/WebInspectorUI/UserInterface/Views/View.js:44
> > +    get isLayoutPending()
> 
> This should be a function if it is prefixed with "is".

What about changing to "has"? We prefix getters with both "is" and "has", the latter being slightly more common.

[Matt Baker]

Created attachment 264740 [details]
[Patch] Proposed Fix

[Timothy Hatcher]

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

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

> Source/WebInspectorUI/UserInterface/Views/View.js:44
> +    get hasPendingLayout()

Getters should have no verb prefix, including "has". Properties should be about the state of the object, not a question being asked. A better property name would be "layoutScheduled" or "layoutPending". Otherwise this should be a function with an "is" or "has" prefix.

[Matt Baker]

Created attachment 264747 [details]
[Patch] Proposed Fix

[Blaze Burg]

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

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

r=me, nice work

> Source/WebInspectorUI/ChangeLog:7
> +

Please add a summary (from the bug is fine).

[Matt Baker]

Created attachment 264878 [details]
[Patch] Proposed Fix

[WebKit Commit Bot]

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

Clearing flags on attachment: 264878

Committed r192070: <http://trac.webkit.org/changeset/192070>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.