MainPageBehavior assumption makes <settings-main> unfriendly to unit testing. |
|||||
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
,
Jul 26 2016
,
Aug 2 2016
,
Aug 2 2016
i've suggested just
this.scroller = this.offsetParent
but we should make sure it works everywhere, tests, hidden pages, etc.
,
Aug 3 2016
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.
,
Dec 10 2016
,
Mar 30 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/affc1ede3eec8077612c7f7fe76792d57854678f commit affc1ede3eec8077612c7f7fe76792d57854678f Author: dpapad <dpapad@chromium.org> Date: Thu Mar 30 20:38:45 2017 MD Settings: Remove paper-header-panel, use IntersectionObserver. BUG= 631281 CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation Review-Url: https://codereview.chromium.org/2184863002 Cr-Commit-Position: refs/heads/master@{#460865} [modify] https://crrev.com/affc1ede3eec8077612c7f7fe76792d57854678f/chrome/browser/resources/settings/settings_page/main_page_behavior.js [modify] https://crrev.com/affc1ede3eec8077612c7f7fe76792d57854678f/chrome/browser/resources/settings/settings_page_css.html [modify] https://crrev.com/affc1ede3eec8077612c7f7fe76792d57854678f/chrome/browser/resources/settings/settings_ui/settings_ui.html [modify] https://crrev.com/affc1ede3eec8077612c7f7fe76792d57854678f/chrome/browser/resources/settings/settings_ui/settings_ui.js
,
Mar 30 2017
paper-header-panel usage has now been removed. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dpa...@chromium.org
, Jul 26 201633.3 KB
33.3 KB View Download