| Summary: | Web Inspector: Add "show more" data grid node and waiting message UI components | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Mikhail Naganov <mnaganov> | ||||||||
| Component: | Web Inspector (Deprecated) | Assignee: | Mikhail Naganov <mnaganov> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | apavlov, bweinstein, joepeck, keishi, loislo, pfeldman, pmuellr, rik, timothy, yurys | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Attachments: |
|
||||||||||
Created attachment 81208 [details]
patch
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 {} 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. Created attachment 81225 [details]
comments addressed
Manually committed http://trac.webkit.org/changeset/77643 2011-02-04 Mikhail Naganov <mnaganov@chromium.org> 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: |
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.