New issue
Advanced search Search tips

Issue 631281 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Mar 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

MainPageBehavior assumption makes <settings-main> unfriendly to unit testing.

Project Member Reported by dpa...@chromium.org, Jul 26 2016

Issue description

I was trying to write some new unit tests for <settings-main> (specifically about "search within settings"). I am hitting a problem with the following method at [1]

/** @override */
attached: function() {
  this.scroller = this.domHost && this.domHost.parentNode.$.mainContainer;
},

MainPageBehavior currently assumes that elements implementing that behavior reside inside a paper-header-panel, see [2]. While in a unit test, this assumption is broken and a runtime error is thrown (see attachment). I also recall that when we transitioned from various paper-* elements to their equivalent app-* elements, I could not figure out how to remove the paper-header-panel parent, without the UI being very broken.

I am primarily looking for a way to make <settings-main> unit testable, and secondarily for a way to move away from paper-header-panel, since we already transitioned other parts of the app to the app-layout elements (for example app-drawer).

[1] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settings_page/main_page_behavior.js?l=44
[2] https://cs.chromium.org/chromium/src/chrome/browser/resources/settings/settings_ui/settings_ui.html?l=131-135
 

Comment 1 by dpa...@chromium.org, Jul 26 2016

Adding the attachment mentioned above.
test_error.png
33.3 KB View Download

Comment 2 by dpa...@chromium.org, Jul 26 2016

Summary: MainPageBehavior assumption makes <settings-main> unfriendly to unit testing. (was: MainPageBehavior assumptions makes <settings-main> unfriendly to unit testing.)
Labels: Hotlist-MD-Settings-General
Status: Available (was: Untriaged)
i've suggested just

    this.scroller = this.offsetParent

but we should make sure it works everywhere, tests, hidden pages, etc.
How about removing paper-header-panel completely? Do we need it? I attempted to do this at https://codereview.chromium.org/2184863002, seems promising, but not sure if it creates any other issues.
Labels: -Pri-2 Pri-3

Comment 8 by dpa...@chromium.org, Mar 30 2017

Status: Fixed (was: Available)
paper-header-panel usage has now been removed.

Sign in to add a comment