I like to sort things by "Total Time". But, every time I open/close or just refresh the inspector, it defaults to sorting things by "Start Time". It would be nice if this preference was remembered.
<rdar://problem/24029666>
Created attachment 268068 [details] Patch
Oh crap sorry @Matt I didn't notice that this was assigned to you. My bad.
No problem.
Comment on attachment 268068 [details] Patch r=me In general I would prefer to give each data grid a well-known id, and let it handle the details of saving to a setting. However, there are no other uses for such an identifier (CSS id comes to mind), so this approach is fine for the time being.
Comment on attachment 268068 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268068&action=review Some comments above. I don't think any immediate concerns though, but long term they should be addressed. > Source/WebInspectorUI/UserInterface/Views/ConsoleMessageView.js:809 > - var dataGrid = WebInspector.DataGrid.createSortableDataGrid(columnNames, flatValues); > + let dataGrid = WebInspector.DataGrid.createSortableDataGrid(columnNames, flatValues, "console-message-view-sort"); This one may be a bit too general. Each `console.table` will have a new Setting with the same conflicting identifier, which would cause conflicts. Not to mention each console.table saving some value to persistent storage if the user changes the sort. Might be best to not include a setting here, or having it just be a Temporary / In Memory Only Setting. (Maybe a new WebInspector.Setting subclass). > Source/WebInspectorUI/UserInterface/Views/DatabaseTableContentView.js:89 > - this._dataGrid = WebInspector.DataGrid.createSortableDataGrid(columnNames, values); > + this._dataGrid = WebInspector.DataGrid.createSortableDataGrid(columnNames, values, "database-table-content-view-sort"); Same here. Unless we wanted to make the identifier more specific, "database-table-content-view-sort-{table-name}". But still, this might be overkill.
Created attachment 268251 [details] Patch So I reverted some of the changes to make it so that the setting object is "optional", in that the DataGrid will still work without it.
Comment on attachment 268251 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268251&action=review r=me! > Source/WebInspectorUI/UserInterface/Views/DataGrid.js:238 > + if (this.sortColumnIdentifier) { > + let newSortHeaderCellElement = this._headerTableCellElements.get(this.sortColumnIdentifier); Since the getter now does nothing these two lines can just access the instance variable directly (_sortColumnIdentifier).
Created attachment 268267 [details] Patch (In reply to comment #8) > > Source/WebInspectorUI/UserInterface/Views/DataGrid.js:238 > > + if (this.sortColumnIdentifier) { > > + let newSortHeaderCellElement = this._headerTableCellElements.get(this.sortColumnIdentifier); > > Since the getter now does nothing these two lines can just access the > instance variable directly (_sortColumnIdentifier). Oops. Fixed it.
Comment on attachment 268267 [details] Patch Clearing flags on attachment: 268267 Committed r194574: <http://trac.webkit.org/changeset/194574>
All reviewed patches have been landed. Closing bug.
Comment on attachment 268267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268267&action=review > Source/WebInspectorUI/UserInterface/Views/ApplicationCacheFrameContentView.js:166 > this._dataGrid.sortOrder = WebInspector.DataGrid.SortOrder.Ascending; You remember the column, but not the sort order. It would be good to remember both.
Comment on attachment 268267 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=268267&action=review >> Source/WebInspectorUI/UserInterface/Views/ApplicationCacheFrameContentView.js:166 >> this._dataGrid.sortOrder = WebInspector.DataGrid.SortOrder.Ascending; > > You remember the column, but not the sort order. It would be good to remember both. Just made another bug for this: https://bugs.webkit.org/show_bug.cgi?id=152811