153733 2016-01-31 19:29:07 -0800 Web Inspector: DataGridNode should support adding go-to arrow buttons to any cell 2016-02-01 13:48:44 -0800 1 1 1 Unclassified WebKit Web Inspector WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 153032 1 mattbaker mattbaker bburg commit-queue graouts joepeck mattbaker nvasilyev timothy webkit-bug-importer oldest_to_newest 1160728 0 mattbaker 2016-01-31 19:29:07 -0800 * SUMMARY DataGridNode should support adding go-to arrow buttons to any cell. TimelineDataGridNode automatically creates go-to arrows for SourceCodeLocation cells, but currently it's not convenient to add go-to arrows to arbitrary cells. As part of the Timelines UI redesign, grids will need cells with arrow buttons for navigating to resources: https://bugs.webkit.org/show_bug.cgi?id=153032. 1160729 1 webkit-bug-importer 2016-01-31 19:29:17 -0800 <rdar://problem/24431813> 1160734 2 270363 mattbaker 2016-01-31 19:40:51 -0800 Created attachment 270363 [Patch] Proposed Fix 1160891 3 270363 bburg 2016-02-01 09:41:10 -0800 Comment on attachment 270363 [Patch] Proposed Fix r=me 1160922 4 270363 commit-queue 2016-02-01 10:22:43 -0800 Comment on attachment 270363 [Patch] Proposed Fix Clearing flags on attachment: 270363 Committed r195966: <http://trac.webkit.org/changeset/195966> 1160923 5 commit-queue 2016-02-01 10:22:45 -0800 All reviewed patches have been landed. Closing bug. 1160969 6 270363 joepeck 2016-02-01 12:26:58 -0800 Comment on attachment 270363 [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=270363&action=review > Source/WebInspectorUI/ChangeLog:10 > + Provide a way to add go-to arrow buttons to any grid cell from within a > + DataGridNode subclass's implementation of createCellContent. Why? I think of DataGrid as a table containing cells. In which case: - Most DataGrids will not require this, so it shouldn't be in the base class. - When there are goto arrows, I prefer they have a sourceCodeLocation and so can provide a tooltip - Placement of the goto arrow may be different depending on what the cell wants. - If a cell manually creates its own goto arrow, the DataGrid's GoToArrowClicked is not triggered. That seems confusing. 1161013 7 mattbaker 2016-02-01 13:48:44 -0800 (In reply to comment #6) > Comment on attachment 270363 [details] > [Patch] Proposed Fix > > View in context: > https://bugs.webkit.org/attachment.cgi?id=270363&action=review > > > Source/WebInspectorUI/ChangeLog:10 > > + Provide a way to add go-to arrow buttons to any grid cell from within a > > + DataGridNode subclass's implementation of createCellContent. > > Why? > > I think of DataGrid as a table containing cells. In which case: > > - Most DataGrids will not require this, so it shouldn't be in the base class. > - When there are goto arrows, I prefer they have a sourceCodeLocation and so > can provide a tooltip Currently goto arrows are only created for SourceCodeLocation's but this will be changing. Cells in the 'Name' column created by ResourceDataGridNode (https://webkit.org/b/153034) needs a goto arrow, but don't have SourceCodeLocations. It's up to the NetworkTimeView/OverviewTimelineView to handle the grid's GoToArrowClicked event and open a content view for the node's resource. This is similar to what the TimelineSidebarPanel does now. > - Placement of the goto arrow may be different depending on what the cell > wants. It seems like right-aligned goto arrows is pretty standard throughout the UI (SourceCodeLocation cells, tree elements). > - If a cell manually creates its own goto arrow, the DataGrid's > GoToArrowClicked is not triggered. That seems confusing. I agree that this is somewhat confusing. 270363 2016-01-31 19:40:51 -0800 2016-02-01 10:22:43 -0800 [Patch] Proposed Fix bug-153733-20160131194027.patch text/plain 2971 mattbaker U3VidmVyc2lvbiBSZXZpc2lvbjogMTk1OTQ4CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViSW5zcGVj dG9yVUkvQ2hhbmdlTG9nIGIvU291cmNlL1dlYkluc3BlY3RvclVJL0NoYW5nZUxvZwppbmRleCAz Njg5NzQ4YWQ5YmJiNzBhM2VjODExNWVjMGIzMTI2YzliYjM5YzFiLi41NGYyNmM3NGUyYTEyMTg0 NDY4YjFjODk2NDExZjFhNzdmZWE0NTE2IDEwMDY0NAotLS0gYS9Tb3VyY2UvV2ViSW5zcGVjdG9y VUkvQ2hhbmdlTG9nCisrKyBiL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9DaGFuZ2VMb2cKQEAgLTEs MyArMSwyMiBAQAorMjAxNi0wMS0zMSAgTWF0dCBCYWtlciAgPG1hdHRiYWtlckBhcHBsZS5jb20+ CisKKyAgICAgICAgV2ViIEluc3BlY3RvcjogRGF0YUdyaWROb2RlIHNob3VsZCBzdXBwb3J0IGFk ZGluZyBnby10byBhcnJvdyBidXR0b25zIHRvIGFueSBjZWxsCisgICAgICAgIGh0dHBzOi8vYnVn cy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xNTM3MzMKKyAgICAgICAgPHJkYXI6Ly9wcm9i bGVtLzI0NDMxODEzPgorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisg ICAgICAgIFByb3ZpZGUgYSB3YXkgdG8gYWRkIGdvLXRvIGFycm93IGJ1dHRvbnMgdG8gYW55IGdy aWQgY2VsbCBmcm9tIHdpdGhpbiBhCisgICAgICAgIERhdGFHcmlkTm9kZSBzdWJjbGFzcydzIGlt cGxlbWVudGF0aW9uIG9mIGNyZWF0ZUNlbGxDb250ZW50LgorCisgICAgICAgICogVXNlckludGVy ZmFjZS9WaWV3cy9EYXRhR3JpZC5qczoKKyAgICAgICAgTmV3IGV2ZW50IHR5cGUsIEdvVG9BcnJv d0NsaWNrZWQuCisgICAgICAgIChXZWJJbnNwZWN0b3IuRGF0YUdyaWROb2RlLnByb3RvdHlwZS5j cmVhdGVHb1RvQXJyb3dCdXR0b24uYnV0dG9uQ2xpY2tlZCk6CisgICAgICAgIChXZWJJbnNwZWN0 b3IuRGF0YUdyaWROb2RlLnByb3RvdHlwZS5jcmVhdGVHb1RvQXJyb3dCdXR0b24pOgorICAgICAg ICBBZGRzIGEgZ28tdG8gYXJyb3cgYnV0dG9uIHRvIHRoZSBjZWxsJ3MgY29udGVudCBlbGVtZW50 LiBDbGlja2luZyB0aGUgYnV0dG9uCisgICAgICAgIGRpc3BhdGNoZXMgYW4gZXZlbnQgb24gdGhl IERhdGFHcmlkLCB3aXRoIGV2ZW50IGRhdGEgY29udGFpbmluZyB0aGUKKyAgICAgICAgRGF0YUdy aWROb2RlIGFuZCBpZGVudGlmaWVyIG9mIHRoZSBjZWxsIGNvbnRhaW5pbmcgdGhlIGFycm93IGJ1 dHRvbi4KKwogMjAxNi0wMS0zMCAgRGF2ZSBIeWF0dCAgPGh5YXR0QGFwcGxlLmNvbT4KIAogICAg ICAgICBBZGQgdGhlIGJyZWFrLSogcHJvcGVydGllcyB0byBXZWIgSW5zcGVjdG9yJ3MgY29tcGxl dGlvbiBzZXQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJJbnNwZWN0b3JVSS9Vc2VySW50ZXJmYWNl L1ZpZXdzL0RhdGFHcmlkLmpzIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2Uv Vmlld3MvRGF0YUdyaWQuanMKaW5kZXggY2I2OGNmZmM2ZDE0OTU4YTJiNjgzYTdmZWJlMGRjYmY0 YTJkYjFkZS4uN2Y3YTU3NWM3MjYxNGJkMWYwZTU5NjE0N2U1NGNjOTY0NTM0YjVlNiAxMDA2NDQK LS0tIGEvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvRGF0YUdyaWQu anMKKysrIGIvU291cmNlL1dlYkluc3BlY3RvclVJL1VzZXJJbnRlcmZhY2UvVmlld3MvRGF0YUdy aWQuanMKQEAgLTEzNDQsNyArMTM0NCw4IEBAIFdlYkluc3BlY3Rvci5EYXRhR3JpZC5FdmVudCA9 IHsKICAgICBTb3J0Q2hhbmdlZDogImRhdGFncmlkLXNvcnQtY2hhbmdlZCIsCiAgICAgU2VsZWN0 ZWROb2RlQ2hhbmdlZDogImRhdGFncmlkLXNlbGVjdGVkLW5vZGUtY2hhbmdlZCIsCiAgICAgRXhw YW5kZWROb2RlOiAiZGF0YWdyaWQtZXhwYW5kZWQtbm9kZSIsCi0gICAgQ29sbGFwc2VkTm9kZTog ImRhdGFncmlkLWNvbGxhcHNlZC1ub2RlIgorICAgIENvbGxhcHNlZE5vZGU6ICJkYXRhZ3JpZC1j b2xsYXBzZWQtbm9kZSIsCisgICAgR29Ub0Fycm93Q2xpY2tlZDogImRhdGFncmlkLWdvLXRvLWFy cm93LWNsaWNrZWQiCiB9OwogCiBXZWJJbnNwZWN0b3IuRGF0YUdyaWQuUmVzaXplTWV0aG9kID0g ewpAQCAtMTQ1NCw2ICsxNDU1LDI2IEBAIFdlYkluc3BlY3Rvci5EYXRhR3JpZE5vZGUgPSBjbGFz cyBEYXRhR3JpZE5vZGUgZXh0ZW5kcyBXZWJJbnNwZWN0b3IuT2JqZWN0CiAgICAgICAgICAgICB0 aGlzLl9lbGVtZW50LmFwcGVuZENoaWxkKHRoaXMuY3JlYXRlQ2VsbChjb2x1bW5JZGVudGlmaWVy KSk7CiAgICAgfQogCisgICAgY3JlYXRlR29Ub0Fycm93QnV0dG9uKGNlbGxFbGVtZW50KQorICAg IHsKKyAgICAgICAgZnVuY3Rpb24gYnV0dG9uQ2xpY2tlZChldmVudCkKKyAgICAgICAgeworICAg ICAgICAgICAgaWYgKHRoaXMuaGlkZGVuIHx8ICF0aGlzLnJldmVhbGVkKQorICAgICAgICAgICAg ICAgIHJldHVybjsKKworICAgICAgICAgICAgZXZlbnQuc3RvcFByb3BhZ2F0aW9uKCk7CisKKyAg ICAgICAgICAgIGxldCBjb2x1bW5JZGVudGlmaWVyID0gY2VsbEVsZW1lbnQuX19jb2x1bW5JZGVu dGlmaWVyOworICAgICAgICAgICAgdGhpcy5kYXRhR3JpZC5kaXNwYXRjaEV2ZW50VG9MaXN0ZW5l cnMoV2ViSW5zcGVjdG9yLkRhdGFHcmlkLkV2ZW50LkdvVG9BcnJvd0NsaWNrZWQsIHtkYXRhR3Jp ZE5vZGU6IHRoaXMsIGNvbHVtbklkZW50aWZpZXJ9KTsKKyAgICAgICAgfQorCisgICAgICAgIGxl dCBidXR0b24gPSBXZWJJbnNwZWN0b3IuY3JlYXRlR29Ub0Fycm93QnV0dG9uKCk7CisgICAgICAg IGJ1dHRvbi5hZGRFdmVudExpc3RlbmVyKCJjbGljayIsIGJ1dHRvbkNsaWNrZWQuYmluZCh0aGlz KSk7CisKKyAgICAgICAgbGV0IGNvbnRlbnRFbGVtZW50ID0gY2VsbEVsZW1lbnQuZmlyc3RDaGls ZDsKKyAgICAgICAgY29udGVudEVsZW1lbnQuYXBwZW5kQ2hpbGQoYnV0dG9uKTsKKyAgICB9CisK ICAgICByZWZyZXNoSWZOZWVkZWQoKQogICAgIHsKICAgICAgICAgaWYgKCF0aGlzLl9uZWVkc1Jl ZnJlc2gpCg==