MD Settings: cr/cros - Guest mode page visibility

chrome/browser/resources/settings/settings_main/settings_main.js

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 /**
  * @fileoverview
  * 'settings-main' displays the selected settings page.
  */
 Polymer({
   is: 'settings-main',
@@ skipping 39 matching lines @@
     showBasicPage_: {
       type: Boolean,
       value: true,
     },
 
     /** @private */
     showAboutPage_: {
       type: Boolean,
       value: false,
     },
+
+    /**
+     * Dictionary defining page visibility.
+     * @type {Object<boolean>}
+     */
+    pageVisibility: {
+      type: Object,
+      value: function() { return {}; },
+    },
   },
 
   /** @override */
   created: function() {
     /** @private {!PromiseResolver} */
     this.resolver_ = new PromiseResolver;
     settings.main.rendered = this.resolver_.promise;
   },
 
   attached: function() {
@@ skipping 38 matching lines @@
 
   /**
    * @param {!SettingsRoute} newRoute
    * @private
    */
   currentRouteChanged_: function(newRoute) {
     var isSubpage = !!newRoute.subpage.length;
 
     this.showAboutPage_ = newRoute.page == 'about';
 
     this.showAdvancedToggle_ = !this.showAboutPage_ && !isSubpage;

[michaelpg, 2016/07/13 00:01:35]

 && !this.pageVisibility.hideAdvancedSettings

[Moe, 2016/07/13 20:17:17]

the logic should probably be more like (which IMO is too much):

currentRouteOrPageVisbilityChanged_: function(newRoute, newPageVisibility) {
  var isSubpage = !!newRoute.subpage.length;
 
  this.showAboutPage_ = newRoute.page == 'about';
 
  var isAboutPageOrSubpage = this.showAboutPage_ || isSubpage;

  this.showAdvancedToggle_ =
      !isAboutPageOrSubpage &&
      this.showPage(newPageVisibility.advancedSettings);
 
  this.showBasicPage_ = !isAboutPageOrSubpage || newRoute.page == 'basic';
 
  this.showAdvancedPage_ =
    ((this.isAdvancedMenuOpen_ && !isAboutPageOrSubpage) ||
     newRoute.page == 'advanced') &&
    this.showPage(newPageVisibility.advancedSettings);
 
  this.style.height = isSubpage ? '100%' : '';
}

[michaelpg, 2016/07/14 00:31:28]

Acknowledged.

 
     this.showBasicPage_ = this.showAdvancedToggle_ || newRoute.page == 'basic';
 
     this.showAdvancedPage_ =
         (this.isAdvancedMenuOpen_ && this.showAdvancedToggle_) ||
         newRoute.page == 'advanced';

[michaelpg, 2016/07/13 00:01:36]

It's probably worth asserting
    !pageVisibility.hideAdvancedSettings
if
    newRoute.page == 'advanced'
here.    

[Moe, 2016/07/13 20:17:17]

do you think this is this still needed if we go with the dom-if?

[michaelpg, 2016/07/14 00:31:28]

Yeah. My reasoning is that asking for an Advanced route should be considered an
exception if Advanced settings are force-hidden.

In guest mode, if we initially land on an advanced page (e.g.
chrome://md-settings/advanced), settings-router should fix that immediately.
After that, the UI shouldn't be allowed to set an Advanced route. (I have no
idea what state the page would get into if you tried.)

[Moe, 2016/07/14 17:19:55]

We should be good here. chrome://md-settings/advanced is an invalid route in cr
guest mode. So we shouldn't get here with newRoute.page == 'advanced'. I'll add
an assert here and one to settings_menu.js where we have a similar situation.

 
     this.style.height = isSubpage ? '100%' : '';
   },
 
   /** @private */
   toggleAdvancedPage_: function() {
     this.fire('toggle-advanced-page', !this.isAdvancedMenuOpen_);
   },
 
   /**
    * Navigates to the default search page (if necessary).
    * @private
    */
   ensureInDefaultSearchPage_: function() {
     if (this.currentRoute.page != 'basic' ||
         this.currentRoute.section != '' ||
         this.currentRoute.subpage.length != 0) {
       this.currentRoute = {page: 'basic', section: '', subpage: [], url: ''};
     }
   },
 
   /**
    * @param {string} query
    */
   searchContents: function(query) {
     this.ensureInDefaultSearchPage_();

[michaelpg, 2016/07/13 00:01:36]

assert !hideSearch
if that's a thing.

[Moe, 2016/07/13 20:17:17]

Done.

 
     // Trigger rendering of the basic and advanced pages and search once ready.
     // Even if those are already rendered, yield to the message loop before
     // initiating searching.
     this.showBasicPage_ = true;
     setTimeout(function() {
       settings.search(query, assert(this.$$('settings-basic-page')));
     }.bind(this), 0);
 
     this.showAdvancedPage_ = true;
     setTimeout(function() {
       settings.search(query, assert(this.$$('settings-advanced-page')));
     }.bind(this), 0);
   },
 });