RESOLVED FIXED 153758
Web Inspector: Memory Timeline View should be responsive / resizable
https://bugs.webkit.org/show_bug.cgi?id=153758
Summary Web Inspector: Memory Timeline View should be responsive / resizable
Joseph Pecoraro
Reported 2016-02-01 13:19:25 PST
* SUMMARY Memory Timeline View should be responsive / resizable. Currently the MemoryCategoryView is fixed at 800/75. In general the Memory Timeline View expects to be 900px wide. It doesn't need to be so wide. Also it might be nice to layout the charts differently at narrow widths.
Attachments
Patch (5.83 KB, patch)
2016-08-15 17:56 PDT, Devin Rousso
no flags
[IMAGE] Issue (173.19 KB, image/png)
2016-08-15 19:39 PDT, Joseph Pecoraro
no flags
Patch (31.79 KB, patch)
2019-01-30 19:08 PST, Devin Rousso
no flags
Patch (31.80 KB, patch)
2019-01-30 19:11 PST, Devin Rousso
no flags
[Image] After Patch is applied (685.15 KB, image/png)
2019-01-30 19:14 PST, Devin Rousso
no flags
Patch (31.71 KB, patch)
2019-01-30 22:15 PST, Devin Rousso
no flags
Radar WebKit Bug Importer
Comment 1 2016-02-01 13:19:58 PST
Devin Rousso
Comment 2 2016-08-15 17:56:51 PDT
Joseph Pecoraro
Comment 3 2016-08-15 19:39:37 PDT
Comment on attachment 286123 [details] Patch Wow this is awesome! It is close to working but doesn't quite work at the moment though, so r- until fixed. Issue: When I resize Web Inspector > 800px the line chart only draws 800px wide but the timeline overview draws as wide as the window 1600px+. Steps to Reproduce: 1. Inspect any page 2. Start Timeline recording with Memory Timeline 3. Record for about 10 seconds then stop 4. Resize Web Inspector > 1000px wide 5. Show Memory Timeline view 6. Select 0-10s in the timeline overview => Line Chart only draws 800px, doesn't match timeline ruler times Also I would expect there to be some padding around the line charts, so that they don't go right up to the edge of the inspector window.
Joseph Pecoraro
Comment 4 2016-08-15 19:39:56 PDT
Created attachment 286137 [details] [IMAGE] Issue
Devin Rousso
Comment 5 2019-01-30 19:08:10 PST
Devin Rousso
Comment 6 2019-01-30 19:11:27 PST
Devin Rousso
Comment 7 2019-01-30 19:14:46 PST
Created attachment 360675 [details] [Image] After Patch is applied
Joseph Pecoraro
Comment 8 2019-01-30 19:43:42 PST
Comment on attachment 360674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360674&action=review r=me > Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:165 > + function xScale(value) { Why rename this from `time` to `value`? > Source/WebInspectorUI/UserInterface/Views/LineChart.js:89 > addPoint(x, y) > { > this._points.push({x, y}); > + > + this.needsLayout(); > } > > clear() > { > this._points = []; > + > + this.needsLayout(); > } I don't like these changes. This is going to mean lots and lots of unnecessary needsLayout calls happening when adding say 1000 points to a chart (which will be the common case).
Devin Rousso
Comment 9 2019-01-30 22:14:53 PST
Comment on attachment 360674 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=360674&action=review >> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:165 >> + function xScale(value) { > > Why rename this from `time` to `value`? No good reason really, more-so to just match the changes to `yScale`. I'll revert. >> Source/WebInspectorUI/UserInterface/Views/LineChart.js:89 >> } > > I don't like these changes. This is going to mean lots and lots of unnecessary needsLayout calls happening when adding say 1000 points to a chart (which will be the common case). Successive calls to `needsLayout` are very early returns, so I don't think this is the worst thing, but you make a good point, so I'll remove these changes.
Devin Rousso
Comment 10 2019-01-30 22:15:03 PST
WebKit Commit Bot
Comment 11 2019-01-30 22:52:40 PST
Comment on attachment 360690 [details] Patch Clearing flags on attachment: 360690 Committed r240763: <https://trac.webkit.org/changeset/240763>
WebKit Commit Bot
Comment 12 2019-01-30 22:52:42 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.