Bug 53763 – Web Inspector: Add "show more" data grid node and waiting message UI components

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

[Mikhail Naganov]

Created attachment 81207 [details]
UI Screenshots

I implemented two UI components:
  - "show more" data grid node is used for on-demand population of data grid contents (similar to DOM tree capability for limiting displayed nodes count);
  - waiting message is used for informing user about long lasting operations (with a possibility to cancel them).

Screenshots are attached.

[Mikhail Naganov]

Created attachment 81208 [details]
patch

[Pavel Feldman]

Comment on attachment 81208 [details]
patch

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

> Source/WebCore/inspector/front-end/DataGrid.js:1480
> +WebInspector.ShowMoreDataGridNode = function(callback, nextCount, allCount)

I think you should place this into a separate file.

> Source/WebCore/inspector/front-end/DataGrid.js:1494
> +    function populate(count)

define above the usage please.

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:35
> +    this.element.textContent = WebInspector.UIString("Please waitâ¦");

Could you use \u notation here?

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:38
> +    this.cancelButton.setAttribute("type", "button");

No need to set button type to button.

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:54
> +WebInspector.PleaseWaitMessage.prototype._instance = new WebInspector.PleaseWaitMessage();

This should be a lazy initializer in order not to compromise load time.

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:56
> +WebInspector.PleaseWaitMessage.prototype.startAction = function(element, actionCallback, cancelCallback)

Please define within prototype's {}

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:65
> +    WebInspector.PleaseWaitMessage.prototype.show(element, cancelCallback);

this.show() ?

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:66
> +    window.setTimeout(doAction, 0);

setTimeout(doAction, 0) (with no window. prefix).

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:68
> +    function doAction()

Define then use.

> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:79
> +WebInspector.PleaseWaitMessage.prototype.show = function(element, cancelCallback)

Define within {}

[Mikhail Naganov]

Comment on attachment 81208 [details]
patch

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

>> Source/WebCore/inspector/front-end/DataGrid.js:1480
>> +WebInspector.ShowMoreDataGridNode = function(callback, nextCount, allCount)
> 
> I think you should place this into a separate file.

Done.

>> Source/WebCore/inspector/front-end/DataGrid.js:1494
>> +    function populate(count)
> 
> define above the usage please.

Done.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:35
>> +    this.element.textContent = WebInspector.UIString("Please waitâ¦");
> 
> Could you use \u notation here?

Fixed.
We will need to teach check-inspector-strings to convert \u literals from source strings to corresponding characters. Otherwise, it can't match this UI string with the one from localizedStrings. I will do it in a separate commit.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:38
>> +    this.cancelButton.setAttribute("type", "button");
> 
> No need to set button type to button.

Fixed.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:54
>> +WebInspector.PleaseWaitMessage.prototype._instance = new WebInspector.PleaseWaitMessage();
> 
> This should be a lazy initializer in order not to compromise load time.

Done.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:56
>> +WebInspector.PleaseWaitMessage.prototype.startAction = function(element, actionCallback, cancelCallback)
> 
> Please define within prototype's {}

Done.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:65
>> +    WebInspector.PleaseWaitMessage.prototype.show(element, cancelCallback);
> 
> this.show() ?

This style of invocation emphasizes that we call a "class method", which doesn't use "this".

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:66
>> +    window.setTimeout(doAction, 0);
> 
> setTimeout(doAction, 0) (with no window. prefix).

Done.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:68
>> +    function doAction()
> 
> Define then use.

Done.

>> Source/WebCore/inspector/front-end/PleaseWaitMessage.js:79
>> +WebInspector.PleaseWaitMessage.prototype.show = function(element, cancelCallback)
> 
> Define within {}

Done.

[Mikhail Naganov]

Created attachment 81225 [details]
comments addressed

[Mikhail Naganov]

Manually committed http://trac.webkit.org/changeset/77643


2011-02-04  Mikhail Naganov  <[e-mail]>

        Reviewed by Pavel Feldman.

        Web Inspector: Add "show more" data grid node and waiting message UI components.
        https://bugs.webkit.org/show_bug.cgi?id=53763

        - "show more" data grid node is used for on-demand population of
        data grid contents (similar to DOM tree capability for limiting
        displayed nodes count);

        - waiting message is used for informing user about long lasting
        operations (with a possibility to cancel them).

        * English.lproj/localizedStrings.js:
        * WebCore.gypi:
        * WebCore.vcproj/WebCore.vcproj:
        * inspector/front-end/PleaseWaitMessage.js: Added.
        (WebInspector.PleaseWaitMessage):
        * inspector/front-end/ShowMoreDataGridNode.js: Added.
        (WebInspector.ShowMoreDataGridNode):
        * inspector/front-end/WebKit.qrc:
        * inspector/front-end/inspector.css:
        (.data-grid button):
        (.please-wait-msg):
        * inspector/front-end/inspector.html: