⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 267038 in webkit


Ignore:
Timestamp:
Sep 14, 2020, 1:51:40 PM (6 years ago)
Author:
commit-queue@webkit.org
Message:

Web Inspector: Stop Recording in Timelines tab doesn't work reliably
​https://bugs.webkit.org/show_bug.cgi?id=216416

Patch by Patrick Angle <Patrick Angle> on 2020-09-14
Reviewed by Devin Rousso.

Improved UX surrounding stopping recordings. In the Frames view, the interface now shows a "Stopping Recording
Timeline Data" message and disables the Stop Recording button as soon as the user has pressed the button to stop
recording. For all timeline views, the Record/Continue/Stop buttons in the top-right are replaced with an
indeterminate progress spinner while recording is being stopped to better indicate that all work has not
completed yet. Lastly, the recording head now stops at the moment recording was stopped. After we have finished
stopping the recording, the timeline is updated one more time to account for any records we receive after the
moment the user requested recording to stop, instead of continously updating during the stopping phase, which
made it appear that recording was instead continuing.

  • Localizations/en.lproj/localizedStrings.js:
  • UserInterface/Main.html: Added IndeterminateProgressSpinnerNavigationItem.css and

IndeterminateProgressSpinnerNavigationItem.js.

  • UserInterface/Views/ButtonNavigationItem.css:

(.navigation-bar .item.button.image-only):

  • UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.css: Added.

(.navigation-bar .item.indeterminate-progress-spinner):
(.navigation-bar .item.indeterminate-progress-spinner-item .indeterminate-progress-spinner):

  • UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.js: Added.

(WI.IndeterminateProgressSpinnerNavigationItem):

  • UserInterface/Views/TextNavigationItem.js: Drive-by fix for misplaced assert.

(WI.TextNavigationItem):

  • UserInterface/Views/TimelineRecordingContentView.js: Don't advanced the recording head after the user requests

recording to stop.
(WI.TimelineRecordingContentView.prototype._updateTimes):
(WI.TimelineRecordingContentView.prototype._updateTimelineViewTimes):

  • UserInterface/Views/TimelineRecordingProgressView.js: Listen to the TimelineManager to show different messages

for Recording and Stopping, as well as disabling the Stop Recording button.
(WI.TimelineRecordingProgressView):
(WI.TimelineRecordingProgressView.prototype.set visible):
(WI.TimelineRecordingProgressView.prototype._updateState):
(WI.TimelineRecordingProgressView.prototype._handleTimelineCapturingStateChanged):

  • UserInterface/Views/TimelineTabContentView.js: Show a progress spinner while stopping recording.

(WI.TimelineTabContentView):
(WI.TimelineTabContentView.prototype._showRecordButton):
(WI.TimelineTabContentView.prototype._showRecordStoppingSpinner):
(WI.TimelineTabContentView.prototype._showContinueButton):
(WI.TimelineTabContentView.prototype._updateNavigationBarButtons):
(WI.TimelineTabContentView.prototype._handleTimelineCapturingStateChanged):

  • UserInterface/Views/Variables.css: New variable for navigation items with only an icon.

(:root):

Location:
trunk/Source/WebInspectorUI
Files:
9 edited
2 copied

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebInspectorUI/ChangeLog

    r267031 r267038  
     12020-09-14  Patrick Angle  <pangle@apple.com>
     2
     3        Web Inspector: Stop Recording in Timelines tab doesn't work reliably
     4        https://bugs.webkit.org/show_bug.cgi?id=216416
     5
     6        Reviewed by Devin Rousso.
     7
     8        Improved UX surrounding stopping recordings. In the Frames view, the interface now shows a "Stopping Recording
     9        Timeline Data" message and disables the Stop Recording button as soon as the user has pressed the button to stop
     10        recording. For all timeline views, the Record/Continue/Stop buttons in the top-right are replaced with an
     11        indeterminate progress spinner while recording is being stopped to better indicate that all work has not
     12        completed yet. Lastly, the recording head now stops at the moment recording was stopped. After we have finished
     13        stopping the recording, the timeline is updated one more time to account for any records we receive after the
     14        moment the user requested recording to stop, instead of continously updating during the stopping phase, which
     15        made it appear that recording was instead continuing.
     16
     17        * Localizations/en.lproj/localizedStrings.js:
     18        * UserInterface/Main.html: Added `IndeterminateProgressSpinnerNavigationItem.css` and
     19        `IndeterminateProgressSpinnerNavigationItem.js`.
     20        * UserInterface/Views/ButtonNavigationItem.css:
     21        (.navigation-bar .item.button.image-only):
     22        * UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.css: Added.
     23        (.navigation-bar .item.indeterminate-progress-spinner):
     24        (.navigation-bar .item.indeterminate-progress-spinner-item .indeterminate-progress-spinner):
     25        * UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.js: Added.
     26        (WI.IndeterminateProgressSpinnerNavigationItem):
     27        * UserInterface/Views/TextNavigationItem.js: Drive-by fix for misplaced `assert`.
     28        (WI.TextNavigationItem):
     29        * UserInterface/Views/TimelineRecordingContentView.js: Don't advanced the recording head after the user requests
     30        recording to stop.
     31        (WI.TimelineRecordingContentView.prototype._updateTimes):
     32        (WI.TimelineRecordingContentView.prototype._updateTimelineViewTimes):
     33        * UserInterface/Views/TimelineRecordingProgressView.js: Listen to the TimelineManager to show different messages
     34        for Recording and Stopping, as well as disabling the Stop Recording button.
     35        (WI.TimelineRecordingProgressView):
     36        (WI.TimelineRecordingProgressView.prototype.set visible):
     37        (WI.TimelineRecordingProgressView.prototype._updateState):
     38        (WI.TimelineRecordingProgressView.prototype._handleTimelineCapturingStateChanged):
     39        * UserInterface/Views/TimelineTabContentView.js: Show a progress spinner while stopping recording.
     40        (WI.TimelineTabContentView):
     41        (WI.TimelineTabContentView.prototype._showRecordButton):
     42        (WI.TimelineTabContentView.prototype._showRecordStoppingSpinner):
     43        (WI.TimelineTabContentView.prototype._showContinueButton):
     44        (WI.TimelineTabContentView.prototype._updateNavigationBarButtons):
     45        (WI.TimelineTabContentView.prototype._handleTimelineCapturingStateChanged):
     46        * UserInterface/Views/Variables.css: New variable for navigation items with only an icon.
     47        (:root):
     48
    1492020-09-14  Devin Rousso  <drousso@apple.com>
    250

  • trunk/Source/WebInspectorUI/Localizations/en.lproj/localizedStrings.js

    r266670 r267038  
    10111011localizedStrings["Recording %d"] = "Recording %d";
    10121012localizedStrings["Recording Error: %s"] = "Recording Error: %s";
    1013 localizedStrings["Recording Timeline Data"] = "Recording Timeline Data";
     1013/* Message for progress of a timeline recording. */
     1014localizedStrings["Recording Timeline Data @ Timeline Recording Progress"] = "Recording Timeline Data";
    10141015/* A type of canvas recording in the Graphics Tab */
    10151016localizedStrings["Recording Type Canvas Bitmap Renderer"] = "Bitmap Renderer";
    … …  
    12541255localizedStrings["Stop recording canvas actions"] = "Stop recording canvas actions";
    12551256localizedStrings["Stop recording once page loads"] = "Stop recording once page loads";
     1257/* Message for progress of stopping a timeline recording. */
     1258localizedStrings["Stopping Timeline Recording @ Timeline Recording Progress"] = "Stopping Timeline Recording";
     1259localizedStrings["Stopping recording"] = "Stopping recording";
    12561260localizedStrings["Stopping the \u201C%s\u201D audit"] = "Stopping the \u201C%s\u201D audit";
    12571261localizedStrings["Storage"] = "Storage";

  • trunk/Source/WebInspectorUI/UserInterface/Main.html

    r266480 r267038  
    124124    <link rel="stylesheet" href="Views/ImageResourceContentView.css">
    125125    <link rel="stylesheet" href="Views/IndeterminateProgressSpinner.css">
     126    <link rel="stylesheet" href="Views/IndeterminateProgressSpinnerNavigationItem.css">
    126127    <link rel="stylesheet" href="Views/IndexedDatabaseContentView.css">
    127128    <link rel="stylesheet" href="Views/IndexedDatabaseObjectStoreContentView.css">
    … …  
    712713    <script src="Views/ImageResourceContentView.js"></script>
    713714    <script src="Views/IndeterminateProgressSpinner.js"></script>
     715    <script src="Views/IndeterminateProgressSpinnerNavigationItem.js"></script>
    714716    <script src="Views/IndexedDatabaseContentView.js"></script>
    715717    <script src="Views/IndexedDatabaseDetailsSidebarPanel.js"></script>

  • trunk/Source/WebInspectorUI/UserInterface/Views/ButtonNavigationItem.css

    r262397 r267038  
    3939
    4040.navigation-bar .item.button.image-only {
    41     width: 26px;
     41    width: var(--image-button-navigation-item-width);
    4242}
    4343

  • trunk/Source/WebInspectorUI/UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.css

    r267037 r267038  
    11/*
    2  * Copyright (C) 2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2020 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2424 */
    2525
    26 WI.TextNavigationItem = class TextNavigationItem extends WI.NavigationItem
    27 {
    28     constructor(identifier, label)
    29     {
    30         super(identifier);
     26.navigation-bar .item.indeterminate-progress-spinner-item {
     27    width: var(--image-button-navigation-item-width);
     28    padding: 1px 4px;
     29}
    3130
    32         console.assert(identifier);
    33 
    34         this._element.classList.add("text");
    35         this._element.textContent = label || "";
    36     }
    37 
    38     // Public
    39 
    40     get text()
    41     {
    42         return this._element.textContent;
    43     }
    44 
    45     set text(x)
    46     {
    47         this._element.textContent = x || "";
    48     }
    49 
    50     // Protected
    51 
    52     get totalMargin()
    53     {
    54         return super.totalMargin + 4; /* .navigation-bar .item.text */
    55     }
    56 };
     31.navigation-bar .item.indeterminate-progress-spinner-item .indeterminate-progress-spinner {
     32    margin: auto;
     33}

  • trunk/Source/WebInspectorUI/UserInterface/Views/IndeterminateProgressSpinnerNavigationItem.js

    r267037 r267038  
    11/*
    2  * Copyright (C) 2016 Apple Inc. All rights reserved.
     2 * Copyright (C) 2020 Apple Inc. All rights reserved.
    33 *
    44 * Redistribution and use in source and binary forms, with or without
    … …  
    2424 */
    2525
    26 WI.TextNavigationItem = class TextNavigationItem extends WI.NavigationItem
     26WI.IndeterminateProgressSpinnerNavigationItem = class IndeterminateProgressSpinnerNavigationItem extends WI.NavigationItem
    2727{
    28     constructor(identifier, label)
     28    constructor(identifier, tooltip)
    2929    {
     30        console.assert(identifier);
     31
    3032        super(identifier);
    3133
    32         console.assert(identifier);
     34        this.element.classList.add("indeterminate-progress-spinner-item");
    3335
    34         this._element.classList.add("text");
    35         this._element.textContent = label || "";
    36     }
     36        this._spinner = new WI.IndeterminateProgressSpinner;
     37        this.element.appendChild(this._spinner.element);
    3738
    38     // Public
    39 
    40     get text()
    41     {
    42         return this._element.textContent;
    43     }
    44 
    45     set text(x)
    46     {
    47         this._element.textContent = x || "";
    48     }
    49 
    50     // Protected
    51 
    52     get totalMargin()
    53     {
    54         return super.totalMargin + 4; /* .navigation-bar .item.text */
     39        this.tooltip = tooltip ?? "";
    5540    }
    5641};

  • trunk/Source/WebInspectorUI/UserInterface/Views/TextNavigationItem.js

    r248818 r267038  
    2828    constructor(identifier, label)
    2929    {
     30        console.assert(identifier);
     31
    3032        super(identifier);
    31 
    32         console.assert(identifier);
    3333
    3434        this._element.classList.add("text");

  • trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingContentView.js

    r261109 r267038  
    464464        }
    465465
    466         this._timelineOverview.endTime = Math.max(endTime, currentTime);
    467 
    468         this._currentTime = currentTime;
    469         this._timelineOverview.currentTime = currentTime;
     466        if (WI.timelineManager.capturingState !== WI.TimelineManager.CapturingState.Stopping) {
     467            // Only update end time while not stopping, otherwise the interface contues scrolling.
     468            this._timelineOverview.endTime = Math.max(endTime, currentTime);
     469
     470            if (WI.timelineManager.capturingState !== WI.TimelineManager.CapturingState.Inactive) {
     471                // Only update current time while active/starting or else the interface continues scrolling.
     472                this._currentTime = currentTime;
     473                this._timelineOverview.currentTime = currentTime;
     474            }
     475        }
    470476
    471477        if (this.currentTimelineView)
    … …  
    850856
    851857        timelineView.startTime = this._timelineOverview.selectionStartTime;
    852         timelineView.currentTime = this._currentTime;
    853         timelineView.endTime = endTime;
     858
     859        if (WI.timelineManager.capturingState !== WI.TimelineManager.CapturingState.Stopping) {
     860            // Only update end time while not stopping, otherwise the interface contues scrolling.
     861            timelineView.endTime = endTime;
     862
     863            if (WI.timelineManager.capturingState !== WI.TimelineManager.CapturingState.Inactive) {
     864                // Only update current time while active/starting or else the interface continues scrolling.
     865                timelineView.currentTime = this._currentTime;
     866            }
     867        }
    854868    }
    855869

  • trunk/Source/WebInspectorUI/UserInterface/Views/TimelineRecordingProgressView.js

    r243024 r267038  
    3232        this.element.classList.add("recording-progress");
    3333
    34         let statusElement = document.createElement("div");
    35         statusElement.classList.add("status");
    36         statusElement.textContent = WI.UIString("Recording Timeline Data");
    37         this.element.append(statusElement);
     34        let statusGroup = this.element.appendChild(document.createElement("div"));
     35        statusGroup.className = "status";
     36
     37        this._statusElement = statusGroup.appendChild(document.createElement("span"));
    3838
    3939        let spinner = new WI.IndeterminateProgressSpinner;
    40         statusElement.append(spinner.element);
     40        statusGroup.appendChild(spinner.element);
    4141
    4242        this._stopRecordingButtonElement = document.createElement("button");
    4343        this._stopRecordingButtonElement.textContent = WI.UIString("Stop Recording");
    4444        this._stopRecordingButtonElement.addEventListener("click", () => WI.timelineManager.stopCapturing());
    45         this.element.append(this._stopRecordingButtonElement);
     45        this.element.appendChild(this._stopRecordingButtonElement);
    4646    }
    4747
    … …  
    6161        this._visible = x;
    6262        this.element.classList.toggle("hidden", !this._visible);
     63
     64        if (this._visible) {
     65            WI.timelineManager.addEventListener(WI.TimelineManager.Event.CapturingStateChanged, this._handleTimelineCapturingStateChanged, this);
     66            this._updateState();
     67        } else
     68            WI.timelineManager.removeEventListener(WI.TimelineManager.Event.CapturingStateChanged, this._handleTimelineCapturingStateChanged, this);
    6369    }
     70
     71    // Private
     72
     73    _updateState() {
     74        switch (WI.timelineManager.capturingState) {
     75        case WI.TimelineManager.CapturingState.Starting:
     76        case WI.TimelineManager.CapturingState.Active:
     77            this._statusElement.textContent = WI.UIString("Recording Timeline Data", "Recording Timeline Data @ Timeline Recording Progress", "Message for progress of a timeline recording.");
     78            this._stopRecordingButtonElement.disabled = false;
     79            break;
     80        case WI.TimelineManager.CapturingState.Stopping:
     81            this._statusElement.textContent = WI.UIString("Stopping Timeline Recording", "Stopping Timeline Recording @ Timeline Recording Progress", "Message for progress of stopping a timeline recording.");
     82            this._stopRecordingButtonElement.disabled = true;
     83            break;
     84        case WI.TimelineManager.CapturingState.Inactive:
     85            // When inactive this view should be hidden by its parent, so keep the state the same to avoid possibly
     86            // flickering to a different state just before the parent hides us.
     87            break;
     88        }
     89    }
     90
     91    _handleTimelineCapturingStateChanged(event)
     92    {
     93        this._updateState();
     94    }
     95
    6496};

  • trunk/Source/WebInspectorUI/UserInterface/Views/TimelineTabContentView.js

    r266537 r267038  
    5050        this._recordButton.addEventListener(WI.ButtonNavigationItem.Event.Clicked, this._recordButtonClicked, this);
    5151
     52        this._recordStoppingSpinner = new WI.IndeterminateProgressSpinnerNavigationItem("record-stopping", WI.UIString("Stopping recording"));
     53        this._recordStoppingSpinner.visibilityPriority = WI.NavigationItem.VisibilityPriority.High;
     54        this._recordStoppingSpinner.hidden = true;
     55
    5256        this._continueButton = new WI.ButtonNavigationItem("record-continue", WI.UIString("Continue without automatically stopping"), "Images/Resume.svg", 13, 13);
    5357        this._continueButton.visibilityPriority = WI.NavigationItem.VisibilityPriority.High;
    … …  
    5660
    5761        this.contentBrowser.navigationBar.insertNavigationItem(this._recordButton, 0);
    58         this.contentBrowser.navigationBar.insertNavigationItem(this._continueButton, 1);
     62        this.contentBrowser.navigationBar.insertNavigationItem(this._recordStoppingSpinner, 1);
     63        this.contentBrowser.navigationBar.insertNavigationItem(this._continueButton, 2);
    5964
    6065        if (WI.sharedApp.isWebDebuggable()) {
    … …  
    6570            viewModeGroup.visibilityPriority = WI.NavigationItem.VisibilityPriority.High;
    6671
    67             this.contentBrowser.navigationBar.insertNavigationItem(viewModeGroup, 2);
     72            this.contentBrowser.navigationBar.insertNavigationItem(viewModeGroup, 3);
    6873            this.contentBrowser.navigationBar.addEventListener(WI.NavigationBar.Event.NavigationItemSelected, this._viewModeSelected, this);
    6974        }
    … …  
    453458    {
    454459        this._recordButton.hidden = false;
     460        this._recordStoppingSpinner.hidden = true;
    455461        this._continueButton.hidden = true;
    456462    }
    457463
     464    _showRecordStoppingSpinner()
     465    {
     466        this._recordButton.hidden = true;
     467        this._recordStoppingSpinner.hidden = false;
     468        this._continueButton.hidden = true;
     469    }
     470
    458471    _showContinueButton()
    459472    {
    460473        this._recordButton.hidden = true;
     474        this._recordStoppingSpinner.hidden = true;
    461475        this._continueButton.hidden = false;
    462476    }
    … …  
    464478    _updateNavigationBarButtons()
    465479    {
    466         if (!WI.modifierKeys.altKey || !WI.timelineManager.willAutoStop())
     480        if (WI.timelineManager.capturingState === WI.TimelineManager.CapturingState.Stopping)
     481            this._showRecordStoppingSpinner();
     482        else if (!WI.modifierKeys.altKey || !WI.timelineManager.willAutoStop())
    467483            this._showRecordButton();
    468484        else
    … …  
    473489    {
    474490        let enabled = WI.timelineManager.capturingState === WI.TimelineManager.CapturingState.Active || WI.timelineManager.capturingState === WI.TimelineManager.CapturingState.Inactive;
    475 
    476         this._toggleRecordingShortcut.disabled = !enabled;
    477         this._toggleNewRecordingShortcut.disabled = !enabled;
     491        let stopping = WI.timelineManager.capturingState === WI.TimelineManager.CapturingState.Stopping;
     492
     493        this._toggleRecordingShortcut.disabled = !enabled || stopping;
     494        this._toggleNewRecordingShortcut.disabled = !enabled || stopping;
    478495
    479496        this._recordButton.toggled = WI.timelineManager.isCapturing();

  • trunk/Source/WebInspectorUI/UserInterface/Views/Variables.css

    r266480 r267038  
    214214
    215215    --reference-page-link-size: 20px;
     216
     217    --image-button-navigation-item-width: 26px;
    216218}
    217219
Note: See TracChangeset for help on using the changeset viewer.