WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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
Details
Formatted Diff
Diff
[IMAGE] Issue
(173.19 KB, image/png)
2016-08-15 19:39 PDT
,
Joseph Pecoraro
no flags
Details
Patch
(31.79 KB, patch)
2019-01-30 19:08 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Patch
(31.80 KB, patch)
2019-01-30 19:11 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
[Image] After Patch is applied
(685.15 KB, image/png)
2019-01-30 19:14 PST
,
Devin Rousso
no flags
Details
Patch
(31.71 KB, patch)
2019-01-30 22:15 PST
,
Devin Rousso
no flags
Details
Formatted Diff
Diff
Show Obsolete
(4)
View All
Add attachment
proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1
2016-02-01 13:19:58 PST
<
rdar://problem/24444320
>
Devin Rousso
Comment 2
2016-08-15 17:56:51 PDT
Created
attachment 286123
[details]
Patch
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
Created
attachment 360672
[details]
Patch
Devin Rousso
Comment 6
2019-01-30 19:11:27 PST
Created
attachment 360674
[details]
Patch
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
Created
attachment 360690
[details]
Patch
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.
Top of Page
Format For Printing
XML
Clone This Bug