* SUMMARY Show in progress message in Timeline views that do not show data until the recording finishes. The JavaScript & Events timeline does not show data until the recording finishes. Currently during a recording the view is just blank. It should show a message saying that it will be populated when the recording finishes.
<rdar://problem/25970771>
<rdar://problem/25814637>
Created attachment 278376 [details] [Video] New UI
Created attachment 278378 [details] [Patch] Proposed Fix
Comment on attachment 278378 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=278378&action=review Why did the video not show any data in the DataGrid after you stopped the recording? > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:39 > + // For ButtonToolbarItem to be styled correctly, its parent must have the "toolbar" class. > + let fakeToolbarElement = document.createElement("div"); > + fakeToolbarElement.classList.add("toolbar"); We should just copy the relevant styles to a TimelineRecordingProgressView.css or some shared place without the toolbar classes. This is fragile. > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:40 > + fakeToolbarElement.append(this._stopRecordingButton.element); I think the stop button should be visually last (at the bottom). > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:70 > + shown() This should be show(), it is an action not a notification. This could be fixed by putting visible state into View, I think we have a bug for that. Maybe add a FIXME. > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:81 > + hidden() Ditto, hide(). > Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:97 > + this._statusMessageElement.textContent = WebInspector.UIString("Recording Timeline Data: %s").format(Number.secondsToString(this._recordingDuration)); I don't feel strongly that we should show the time here. It is apparent in top ruler. I think the label, spinner and stop button is enough. Then you wouldn't need to call this update method in set recordingDuration. > Source/WebInspectorUI/UserInterface/Views/TimelineView.js:50 > + get showsLiveRecordingData() > + { > + // Implemented by sub-classes if needed. > + return false; > + } LayoutTimelineView, NetworkTimelineView both support live recording data. They should return true. Maybe the default should flip?
Comment on attachment 278378 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=278378&action=review >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:40 >> + fakeToolbarElement.append(this._stopRecordingButton.element); > > I think the stop button should be visually last (at the bottom). I'll try it out. >> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:70 >> + shown() > > This should be show(), it is an action not a notification. This could be fixed by putting visible state into View, I think we have a bug for that. Maybe add a FIXME. I agree, but intentionally chose the naming to be consistent with our code base. Going forward it should be View.show and View.hide. Will add a FIXME.
Comment on attachment 278378 [details] [Patch] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=278378&action=review >>> Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js:70 >>> + shown() >> >> This should be show(), it is an action not a notification. This could be fixed by putting visible state into View, I think we have a bug for that. Maybe add a FIXME. > > I agree, but intentionally chose the naming to be consistent with our code base. Going forward it should be View.show and View.hide. Will add a FIXME. Calling shown() or hidden() on a ContentView does not show/hide them. They are notification methods for subclasses to do work when they do get shown or hidden. A visible setter is more inline with ContentView and SidebarPanel.
Created attachment 278388 [details] [Video] New UI 2 Removed duration, moved button below status message
Created attachment 278389 [details] [Patch] Proposed Fix
Comment on attachment 278389 [details] [Patch] Proposed Fix Nice.
Comment on attachment 278389 [details] [Patch] Proposed Fix Clearing flags on attachment: 278389 Committed r200573: <http://trac.webkit.org/changeset/200573>
All reviewed patches have been landed. Closing bug.
Whoa, the videos for this look awesome! What happens when you start/stop a recording multiple times. Whenever we are recording does it swap out to this view?
(In reply to comment #13) > Whoa, the videos for this look awesome! > > What happens when you start/stop a recording multiple times. Whenever we are > recording does it swap out to this view? Yep.