Chromium Code Reviews Chromium Code Reviews
Issue 2223083003:
MD Settings: Skip hidden nodes when searching. (Closed)
Base URL: https://chromium.googlesource.com/chromium/src.git@fix_subpage_assertion| Index: chrome/browser/resources/settings/search_settings.js |
| diff --git a/chrome/browser/resources/settings/search_settings.js b/chrome/browser/resources/settings/search_settings.js |
| index 8d657744d5d7711dcffc09b4c71950dab5eb0689..0a19af9ea06c8119aa9c64d69853a0abae9e45ea 100644 |
| --- a/chrome/browser/resources/settings/search_settings.js |
| +++ b/chrome/browser/resources/settings/search_settings.js |
| @@ -13,12 +13,19 @@ cr.define('settings', function() { |
| var SEARCH_BUBBLE_CSS_CLASS = 'search-bubble'; |
| /** |
| - * A CSS attribute indicating that a node shoud be ignored during searching. |
| + * A CSS attribute indicating that a node should be ignored during searching. |
| * @const {string} |
| */ |
| var SKIP_SEARCH_CSS_ATTRIBUTE = 'no-search'; |
| /** |
| + * A CSS class used for hiding a SETTINGS-SECTION for the purposes of |
| + * searching. |
| + * @const {string} |
| + */ |
| + var HIDDEN_FOR_SEARCH_CSS_CLASS = 'hidden-for-search'; |
| + |
| + /** |
| * List of elements types that should not be searched at all. |
| * The only DOM-MODULE node is in <body> which is not searched, therefore |
| * DOM-MODULE is not needed in this set. |
| @@ -129,7 +136,9 @@ cr.define('settings', function() { |
| } |
| if (IGNORED_ELEMENTS.has(node.nodeName) || |
| - (node.hasAttribute && node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE))) { |
| + (node.hasAttribute && (node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE) || |
| + node.hasAttribute('hidden') || |
| + node.style.display == 'none'))) { |
|
Dan Beam
2016/08/09 22:01:38
nit: maybe something like
if (IGNORED_ELEMENTS.ha
dpapad
2016/08/09 23:03:02
Done. Except that I am not using 'let'. I have not
|
| return; |
| } |
| @@ -214,7 +223,7 @@ cr.define('settings', function() { |
| } |
| } |
| if (parent) |
| - parent.hidden = false; |
| + parent.classList.remove(HIDDEN_FOR_SEARCH_CSS_CLASS); |
| // Need to add the search bubble after the parent SETTINGS-SECTION has |
| // become visible, otherwise |offsetWidth| returns zero. |
| @@ -334,8 +343,11 @@ cr.define('settings', function() { |
| setSectionsVisibility_: function(visible) { |
| var sections = Polymer.dom( |
| this.node.root).querySelectorAll('settings-section'); |
| - for (var i = 0; i < sections.length; i++) |
| - sections[i].hidden = !visible; |
| + for (var i = 0; i < sections.length; i++) { |
| + visible ? |
|
Dan Beam
2016/08/09 22:01:38
haven't we already talked about these?
dpapad
2016/08/09 23:03:02
Done. classList#toggle() accepts a second paramete
Dan Beam
2016/08/09 23:05:38
it's already covered by "BE CONSISTENT"
|
| + sections[i].classList.remove(HIDDEN_FOR_SEARCH_CSS_CLASS) : |
| + sections[i].classList.add(HIDDEN_FOR_SEARCH_CSS_CLASS); |
| + } |
| }, |
| }; |
