Web Inspector: Make it easy to dynamically show/hide a SettingsView
authormattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 May 2017 20:49:23 +0000 (20:49 +0000)
committermattbaker@apple.com <mattbaker@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 May 2017 20:49:23 +0000 (20:49 +0000)
https://bugs.webkit.org/show_bug.cgi?id=171765
<rdar://problem/32031280>

Reviewed by Brian Burg.

This patch adds SettingsTabContentView.prototype.setSettingsViewVisible,
for dynamically showing/hiding a child view (and its NavigationBar item).
The following new behavior is relevant when more that one child SettingsView
exist in the Settings tab:
 - Hiding the selected view will cause a new view to become selected.
   The previous visible view is selected, if it exists. Otherwise the
   next visible view is used.
 - Showing a view when no views are selected cause the view to be selected.

As the Settings tab currently has only one child view, the behavior above
was tested by adding a handful of vanilla SettingsView objects to the tab
and toggling their visibility.

* UserInterface/Views/NavigationBar.js:
Simplify overloaded parameter `navigationItemOrIdentifierOrIndex`, which
is used in a few places and is always an instance of NavigationItem.

(WebInspector.NavigationBar):
(WebInspector.NavigationBar.prototype.removeNavigationItem):
(WebInspector.NavigationBar.prototype.get selectedNavigationItem):
(WebInspector.NavigationBar.prototype.set selectedNavigationItem):
(WebInspector.NavigationBar.prototype.findNavigationItem):
Lookup a navigation item by its identifier.
(WebInspector.NavigationBar.prototype._findNavigationItem): Deleted.
Replaced overloaded private method with new public method.

* UserInterface/Views/NavigationItem.js:
Cleanup.
(WebInspector.NavigationItem):
(WebInspector.NavigationItem.prototype.get identifier):
(WebInspector.NavigationItem.prototype.get element):
(WebInspector.NavigationItem.prototype.get minimumWidth):
(WebInspector.NavigationItem.prototype.get parentNavigationBar):

* UserInterface/Views/SettingsTabContentView.css:
Use `visibility: hidden` instead of `display: none` when hiding the
NavigationBar, so that the selected view's top position stays the same.

(.content-view.settings):
(.content-view.settings .navigation-bar.invisible):

* UserInterface/Views/SettingsTabContentView.js:
(WebInspector.SettingsTabContentView):
Switch to an array of SettingsViews instead of a map. Fast lookup isn't
a concern due to the small number of items, and having indices simplifies
traversing the previous/next items in `setSettingsViewVisible`.

(WebInspector.SettingsTabContentView.prototype.set selectedSettingsView):
Rename `page` to `settingsView`.
(WebInspector.SettingsTabContentView.prototype.addSettingsView):
(WebInspector.SettingsTabContentView.prototype.setSettingsViewVisible):
Shows/hides the specified view. Hiding the selected SettingsView will
cause another visible view to become selected, if one exists.

(WebInspector.SettingsTabContentView.prototype._updateNavigationBarVisibility):
Helper for updating navigation bar visibility after making a change
to the navigation items.

(WebInspector.SettingsTabContentView.prototype._navigationItemSelected):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@216451 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebInspectorUI/ChangeLog
Source/WebInspectorUI/UserInterface/Views/NavigationBar.js
Source/WebInspectorUI/UserInterface/Views/NavigationItem.js
Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.css
Source/WebInspectorUI/UserInterface/Views/SettingsTabContentView.js

index 7c04cf3..20bde29 100644 (file)
@@ -1,3 +1,71 @@
+2017-05-08  Matt Baker  <mattbaker@apple.com>
+
+        Web Inspector: Make it easy to dynamically show/hide a SettingsView
+        https://bugs.webkit.org/show_bug.cgi?id=171765
+        <rdar://problem/32031280>
+
+        Reviewed by Brian Burg.
+
+        This patch adds SettingsTabContentView.prototype.setSettingsViewVisible,
+        for dynamically showing/hiding a child view (and its NavigationBar item).
+        The following new behavior is relevant when more that one child SettingsView
+        exist in the Settings tab:
+         - Hiding the selected view will cause a new view to become selected.
+           The previous visible view is selected, if it exists. Otherwise the
+           next visible view is used.
+         - Showing a view when no views are selected cause the view to be selected.
+
+        As the Settings tab currently has only one child view, the behavior above
+        was tested by adding a handful of vanilla SettingsView objects to the tab
+        and toggling their visibility.
+
+        * UserInterface/Views/NavigationBar.js:
+        Simplify overloaded parameter `navigationItemOrIdentifierOrIndex`, which
+        is used in a few places and is always an instance of NavigationItem.
+
+        (WebInspector.NavigationBar):
+        (WebInspector.NavigationBar.prototype.removeNavigationItem):
+        (WebInspector.NavigationBar.prototype.get selectedNavigationItem):
+        (WebInspector.NavigationBar.prototype.set selectedNavigationItem):
+        (WebInspector.NavigationBar.prototype.findNavigationItem):
+        Lookup a navigation item by its identifier.
+        (WebInspector.NavigationBar.prototype._findNavigationItem): Deleted.
+        Replaced overloaded private method with new public method.
+
+        * UserInterface/Views/NavigationItem.js:
+        Cleanup.
+        (WebInspector.NavigationItem):
+        (WebInspector.NavigationItem.prototype.get identifier):
+        (WebInspector.NavigationItem.prototype.get element):
+        (WebInspector.NavigationItem.prototype.get minimumWidth):
+        (WebInspector.NavigationItem.prototype.get parentNavigationBar):
+
+        * UserInterface/Views/SettingsTabContentView.css:
+        Use `visibility: hidden` instead of `display: none` when hiding the
+        NavigationBar, so that the selected view's top position stays the same.
+
+        (.content-view.settings):
+        (.content-view.settings .navigation-bar.invisible):
+
+        * UserInterface/Views/SettingsTabContentView.js:
+        (WebInspector.SettingsTabContentView):
+        Switch to an array of SettingsViews instead of a map. Fast lookup isn't
+        a concern due to the small number of items, and having indices simplifies
+        traversing the previous/next items in `setSettingsViewVisible`.
+
+        (WebInspector.SettingsTabContentView.prototype.set selectedSettingsView):
+        Rename `page` to `settingsView`.
+        (WebInspector.SettingsTabContentView.prototype.addSettingsView):
+        (WebInspector.SettingsTabContentView.prototype.setSettingsViewVisible):
+        Shows/hides the specified view. Hiding the selected SettingsView will
+        cause another visible view to become selected, if one exists.
+
+        (WebInspector.SettingsTabContentView.prototype._updateNavigationBarVisibility):
+        Helper for updating navigation bar visibility after making a change
+        to the navigation items.
+
+        (WebInspector.SettingsTabContentView.prototype._navigationItemSelected):
+
 2017-05-08  Myles C. Maxfield  <mmaxfield@apple.com>
 
         Unprefix unicode-bidi CSS values
index 382c367..aef0ad8 100644 (file)
@@ -45,6 +45,7 @@ WebInspector.NavigationBar = class NavigationBar extends WebInspector.View
         this._forceLayout = false;
         this._minimumWidth = NaN;
         this._navigationItems = [];
+        this._selectedNavigationItem = null;
 
         if (navigationItems) {
             for (var i = 0; i < navigationItems.length; ++i)
@@ -92,10 +93,17 @@ WebInspector.NavigationBar = class NavigationBar extends WebInspector.View
         return navigationItem;
     }
 
-    removeNavigationItem(navigationItemOrIdentifierOrIndex)
+    removeNavigationItem(navigationItem)
     {
-        var navigationItem = this._findNavigationItem(navigationItemOrIdentifierOrIndex);
-        if (!navigationItem)
+        console.assert(navigationItem instanceof WebInspector.NavigationItem);
+        if (!(navigationItem instanceof WebInspector.NavigationItem))
+            return null;
+
+        if (!navigationItem._parentNavigationBar)
+            return null;
+
+        console.assert(navigationItem._parentNavigationBar === this, "Cannot remove item with unexpected parent bar.", navigationItem);
+        if (navigationItem._parentNavigationBar !== this)
             return null;
 
         navigationItem._parentNavigationBar = null;
@@ -115,12 +123,15 @@ WebInspector.NavigationBar = class NavigationBar extends WebInspector.View
 
     get selectedNavigationItem()
     {
-        return this._selectedNavigationItem || null;
+        return this._selectedNavigationItem;
     }
 
-    set selectedNavigationItem(navigationItemOrIdentifierOrIndex)
+    set selectedNavigationItem(navigationItem)
     {
-        var navigationItem = this._findNavigationItem(navigationItemOrIdentifierOrIndex);
+        if (navigationItem && navigationItem.parentNavigationBar !== this) {
+            console.error("Cannot select item with unexpected parent bar.", navigationItem);
+            return;
+        }
 
         // Only radio navigation items can be selected.
         if (!(navigationItem instanceof WebInspector.RadioButtonNavigationItem))
@@ -161,6 +172,11 @@ WebInspector.NavigationBar = class NavigationBar extends WebInspector.View
         return false;
     }
 
+    findNavigationItem(identifier)
+    {
+        return this._navigationItems.find((item) => item.identifier === identifier) || null;
+    }
+
     needsLayout()
     {
         this._forceLayout = true;
@@ -204,27 +220,6 @@ WebInspector.NavigationBar = class NavigationBar extends WebInspector.View
 
     // Private
 
-    _findNavigationItem(navigationItemOrIdentifierOrIndex)
-    {
-        var navigationItem = null;
-
-        if (navigationItemOrIdentifierOrIndex instanceof WebInspector.NavigationItem) {
-            if (this._navigationItems.includes(navigationItemOrIdentifierOrIndex))
-                navigationItem = navigationItemOrIdentifierOrIndex;
-        } else if (typeof navigationItemOrIdentifierOrIndex === "number") {
-            navigationItem = this._navigationItems[navigationItemOrIdentifierOrIndex];
-        } else if (typeof navigationItemOrIdentifierOrIndex === "string") {
-            for (var i = 0; i < this._navigationItems.length; ++i) {
-                if (this._navigationItems[i].identifier === navigationItemOrIdentifierOrIndex) {
-                    navigationItem = this._navigationItems[i];
-                    break;
-                }
-            }
-        }
-
-        return navigationItem;
-    }
-
     _mouseDown(event)
     {
         // Only handle left mouse clicks.
index 87bfdfa..33a7ba4 100644 (file)
@@ -33,6 +33,7 @@ WebInspector.NavigationItem = class NavigationItem extends WebInspector.Object
 
         this._element = document.createElement("div");
         this._hidden = false;
+        this._parentNavigationBar = null;
 
         if (role)
             this._element.setAttribute("role", role);
@@ -45,25 +46,10 @@ WebInspector.NavigationItem = class NavigationItem extends WebInspector.Object
 
     // Public
 
-    get identifier()
-    {
-        return this._identifier;
-    }
-
-    get element()
-    {
-        return this._element;
-    }
-
-    get parentNavigationBar()
-    {
-        return this._parentNavigationBar;
-    }
-
-    get minimumWidth()
-    {
-        return this._element.realOffsetWidth;
-    }
+    get identifier() { return this._identifier; }
+    get element() { return this._element; }
+    get minimumWidth() { return this._element.realOffsetWidth; }
+    get parentNavigationBar() { return this._parentNavigationBar; }
 
     updateLayout(expandOnly)
     {
index da161e3..e2b839c 100644 (file)
  */
 
 .content-view.settings {
-    padding: 6vh 0;
+    padding-bottom: 6vh;
     overflow-y: auto;
 }
 
+.content-view.settings .navigation-bar.invisible {
+    visibility: hidden;
+}
+
 .content-view.settings .navigation-bar .item.radio.button.text-only {
     color: inherit;
     background-color: inherit;
index 6188952..51a1641 100644 (file)
@@ -40,11 +40,10 @@ WebInspector.SettingsTabContentView = class SettingsTabContentView extends WebIn
         WebInspector.settings.zoomFactor.addEventListener(WebInspector.Setting.Event.Changed, boundNeedsLayout);
 
         this._navigationBar = new WebInspector.NavigationBar;
-        this._navigationBar.element.classList.add("hidden");
         this._navigationBar.addEventListener(WebInspector.NavigationBar.Event.NavigationItemSelected, this._navigationItemSelected, this);
 
         this._selectedSettingsView = null;
-        this._settingsViewIdentifierMap = new Map;
+        this._settingsViews = [];
 
         this.addSubview(this._navigationBar);
 
@@ -81,46 +80,114 @@ WebInspector.SettingsTabContentView = class SettingsTabContentView extends WebIn
         return this._selectedSettingsView;
     }
 
-    set selectedSettingsView(page)
+    set selectedSettingsView(settingsView)
     {
-        if (this._selectedSettingsView === page)
+        if (this._selectedSettingsView === settingsView)
             return;
 
         if (this._selectedSettingsView)
-            this.replaceSubview(this._selectedSettingsView, page);
+            this.replaceSubview(this._selectedSettingsView, settingsView);
         else
-            this.addSubview(page);
+            this.addSubview(settingsView);
 
-        this._selectedSettingsView = page;
+        this._selectedSettingsView = settingsView;
         this._selectedSettingsView.updateLayout();
 
-        this._navigationBar.selectedNavigationItem = page.identifier;
+        let navigationItem = this._navigationBar.findNavigationItem(settingsView.identifier);
+        console.assert(navigationItem, "Missing navigation item for settings view.", settingsView)
+        if (!navigationItem)
+            return;
+
+        this._navigationBar.selectedNavigationItem = navigationItem;
     }
 
     addSettingsView(settingsView)
     {
-        let identifier = settingsView.identifier;
-        console.assert(!this._settingsViewIdentifierMap.has(identifier), "SettingsView already exists.", settingsView);
-        if (this._settingsViewIdentifierMap.has(identifier))
+        if (this._settingsViews.includes(settingsView)) {
+            console.assert(false, "SettingsView already exists.", settingsView);
+            return;
+        }
+
+        this._settingsViews.push(settingsView);
+        this._navigationBar.addNavigationItem(new WebInspector.RadioButtonNavigationItem(settingsView.identifier, settingsView.displayName));
+
+        this._updateNavigationBarVisibility();
+    }
+
+    setSettingsViewVisible(settingsView, visible)
+    {
+        let navigationItem = this._navigationBar.findNavigationItem(settingsView.identifier);
+        console.assert(navigationItem, "Missing NavigationItem for identifier: " + settingsView.identifier);
+        if (!navigationItem)
+            return;
+
+        if (navigationItem.hidden === !visible)
+            return;
+
+        navigationItem.hidden = !visible;
+        settingsView.element.classList.toggle("hidden", !visible);
+
+        this._updateNavigationBarVisibility();
+
+        if (!this.selectedSettingsView) {
+            if (visible)
+                this.selectedSettingsView = settingsView;
             return;
+        }
 
-        this._settingsViewIdentifierMap.set(identifier, settingsView);
+        if (this.selectedSettingsView !== settingsView)
+            return;
 
-        this._navigationBar.addNavigationItem(new WebInspector.RadioButtonNavigationItem(identifier, settingsView.displayName));
+        let index = this._settingsViews.indexOf(settingsView);
+        console.assert(index !== -1, "SettingsView not found.", settingsView)
+        if (index === -1)
+            return;
+
+        let previousIndex = index;
+        while (--previousIndex >= 0) {
+            let previousNavigationItem = this._navigationBar.navigationItems[previousIndex];
+            console.assert(previousNavigationItem);
+            if (!previousNavigationItem || previousNavigationItem.hidden)
+                continue;
 
-        if (this._settingsViewIdentifierMap.size > 1)
-            this._navigationBar.element.classList.remove("hidden");
+            this.selectedSettingsView = this._settingsViews[previousIndex];
+            return;
+        }
+
+        let nextIndex = index;
+        while (++nextIndex < this._settingsViews.length) {
+            let nextNavigationItem = this._navigationBar.navigationItems[nextIndex];
+            console.assert(nextNavigationItem);
+            if (!nextNavigationItem || nextNavigationItem.hidden)
+                continue;
+
+            this.selectedSettingsView = this._settingsViews[nextIndex];
+            return;
+        }
     }
 
     // Private
 
+    _updateNavigationBarVisibility()
+    {
+        let visibleItems = 0;
+        for (let item of this._navigationBar.navigationItems) {
+            if (!item.hidden && ++visibleItems > 1) {
+                this._navigationBar.element.classList.remove("invisible");
+                return;
+            }
+        }
+
+        this._navigationBar.element.classList.add("invisible");
+    }
+
     _navigationItemSelected(event)
     {
         let navigationItem = event.target.selectedNavigationItem;
         if (!navigationItem)
             return;
 
-        let settingsView = this._settingsViewIdentifierMap.get(navigationItem.identifier);
+        let settingsView = this._settingsViews.find((view) => view.identifier === navigationItem.identifier);
         console.assert(settingsView, "Missing SettingsView for identifier " + navigationItem.identifier);
         if (!settingsView)
             return;