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| OLD | NEW |
|---|---|
| 1 // Copyright 2016 The Chromium Authors. All rights reserved. | 1 // Copyright 2016 The Chromium Authors. All rights reserved. |
| 2 // Use of this source code is governed by a BSD-style license that can be | 2 // Use of this source code is governed by a BSD-style license that can be |
| 3 // found in the LICENSE file. | 3 // found in the LICENSE file. |
| 4 | 4 |
| 5 cr.define('settings', function() { | 5 cr.define('settings', function() { |
| 6 /** @const {string} */ | 6 /** @const {string} */ |
| 7 var WRAPPER_CSS_CLASS = 'search-highlight-wrapper'; | 7 var WRAPPER_CSS_CLASS = 'search-highlight-wrapper'; |
| 8 | 8 |
| 9 /** @const {string} */ | 9 /** @const {string} */ |
| 10 var HIT_CSS_CLASS = 'search-highlight-hit'; | 10 var HIT_CSS_CLASS = 'search-highlight-hit'; |
| 11 | 11 |
| 12 /** @const {string} */ | 12 /** @const {string} */ |
| 13 var SEARCH_BUBBLE_CSS_CLASS = 'search-bubble'; | 13 var SEARCH_BUBBLE_CSS_CLASS = 'search-bubble'; |
| 14 | 14 |
| 15 /** | 15 /** |
| 16 * A CSS attribute indicating that a node shoud be ignored during searching. | 16 * A CSS attribute indicating that a node should be ignored during searching. |
| 17 * @const {string} | 17 * @const {string} |
| 18 */ | 18 */ |
| 19 var SKIP_SEARCH_CSS_ATTRIBUTE = 'no-search'; | 19 var SKIP_SEARCH_CSS_ATTRIBUTE = 'no-search'; |
| 20 | 20 |
| 21 /** | 21 /** |
| 22 * A CSS class used for hiding a SETTINGS-SECTION for the purposes of | |
| 23 * searching. | |
| 24 * @const {string} | |
| 25 */ | |
| 26 var HIDDEN_FOR_SEARCH_CSS_CLASS = 'hidden-for-search'; | |
| 27 | |
| 28 /** | |
| 22 * List of elements types that should not be searched at all. | 29 * List of elements types that should not be searched at all. |
| 23 * The only DOM-MODULE node is in <body> which is not searched, therefore | 30 * The only DOM-MODULE node is in <body> which is not searched, therefore |
| 24 * DOM-MODULE is not needed in this set. | 31 * DOM-MODULE is not needed in this set. |
| 25 * @const {!Set<string>} | 32 * @const {!Set<string>} |
| 26 */ | 33 */ |
| 27 var IGNORED_ELEMENTS = new Set([ | 34 var IGNORED_ELEMENTS = new Set([ |
| 28 'CONTENT', | 35 'CONTENT', |
| 29 'CR-EVENTS', | 36 'CR-EVENTS', |
| 30 'IMG', | 37 'IMG', |
| 31 'IRON-ICON', | 38 'IRON-ICON', |
| (...skipping 90 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
| 122 var foundMatches = false; | 129 var foundMatches = false; |
| 123 function doSearch(node) { | 130 function doSearch(node) { |
| 124 if (node.nodeName == 'TEMPLATE' && node.hasAttribute('name') && | 131 if (node.nodeName == 'TEMPLATE' && node.hasAttribute('name') && |
| 125 !node.if && !node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE)) { | 132 !node.if && !node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE)) { |
| 126 getSearchManager().queue_.addRenderTask( | 133 getSearchManager().queue_.addRenderTask( |
| 127 new RenderTask(request, node)); | 134 new RenderTask(request, node)); |
| 128 return; | 135 return; |
| 129 } | 136 } |
| 130 | 137 |
| 131 if (IGNORED_ELEMENTS.has(node.nodeName) || | 138 if (IGNORED_ELEMENTS.has(node.nodeName) || |
| 132 (node.hasAttribute && node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE))) { | 139 (node.hasAttribute && (node.hasAttribute(SKIP_SEARCH_CSS_ATTRIBUTE) || |
| 140 node.hasAttribute('hidden') || | |
| 141 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
| |
| 133 return; | 142 return; |
| 134 } | 143 } |
| 135 | 144 |
| 136 if (node.nodeType == Node.TEXT_NODE) { | 145 if (node.nodeType == Node.TEXT_NODE) { |
| 137 var textContent = node.nodeValue.trim(); | 146 var textContent = node.nodeValue.trim(); |
| 138 if (textContent.length == 0) | 147 if (textContent.length == 0) |
| 139 return; | 148 return; |
| 140 | 149 |
| 141 if (request.regExp.test(textContent)) { | 150 if (request.regExp.test(textContent)) { |
| 142 foundMatches = true; | 151 foundMatches = true; |
| (...skipping 64 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
| 207 parent.host : parent.parentNode; | 216 parent.host : parent.parentNode; |
| 208 if (parent.nodeName == 'SETTINGS-SUBPAGE') { | 217 if (parent.nodeName == 'SETTINGS-SUBPAGE') { |
| 209 // TODO(dpapad): Cast to SettingsSubpageElement here. | 218 // TODO(dpapad): Cast to SettingsSubpageElement here. |
| 210 associatedControl = assert( | 219 associatedControl = assert( |
| 211 parent.associatedControl, | 220 parent.associatedControl, |
| 212 'An associated control was expected for SETTINGS-SUBPAGE ' + | 221 'An associated control was expected for SETTINGS-SUBPAGE ' + |
| 213 parent.pageTitle + ', but was not found.'); | 222 parent.pageTitle + ', but was not found.'); |
| 214 } | 223 } |
| 215 } | 224 } |
| 216 if (parent) | 225 if (parent) |
| 217 parent.hidden = false; | 226 parent.classList.remove(HIDDEN_FOR_SEARCH_CSS_CLASS); |
| 218 | 227 |
| 219 // Need to add the search bubble after the parent SETTINGS-SECTION has | 228 // Need to add the search bubble after the parent SETTINGS-SECTION has |
| 220 // become visible, otherwise |offsetWidth| returns zero. | 229 // become visible, otherwise |offsetWidth| returns zero. |
| 221 if (associatedControl) | 230 if (associatedControl) |
| 222 highlightAssociatedControl_(associatedControl, rawQuery); | 231 highlightAssociatedControl_(associatedControl, rawQuery); |
| 223 } | 232 } |
| 224 | 233 |
| 225 /** | 234 /** |
| 226 * @constructor | 235 * @constructor |
| 227 * | 236 * |
| (...skipping 99 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
| 327 return Promise.resolve(); | 336 return Promise.resolve(); |
| 328 }, | 337 }, |
| 329 | 338 |
| 330 /** | 339 /** |
| 331 * @param {boolean} visible | 340 * @param {boolean} visible |
| 332 * @private | 341 * @private |
| 333 */ | 342 */ |
| 334 setSectionsVisibility_: function(visible) { | 343 setSectionsVisibility_: function(visible) { |
| 335 var sections = Polymer.dom( | 344 var sections = Polymer.dom( |
| 336 this.node.root).querySelectorAll('settings-section'); | 345 this.node.root).querySelectorAll('settings-section'); |
| 337 for (var i = 0; i < sections.length; i++) | 346 for (var i = 0; i < sections.length; i++) { |
| 338 sections[i].hidden = !visible; | 347 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"
| |
| 348 sections[i].classList.remove(HIDDEN_FOR_SEARCH_CSS_CLASS) : | |
| 349 sections[i].classList.add(HIDDEN_FOR_SEARCH_CSS_CLASS); | |
| 350 } | |
| 339 }, | 351 }, |
| 340 }; | 352 }; |
| 341 | 353 |
| 342 /** | 354 /** |
| 343 * @constructor | 355 * @constructor |
| 344 */ | 356 */ |
| 345 function TaskQueue() { | 357 function TaskQueue() { |
| 346 /** | 358 /** |
| 347 * @private {{ | 359 * @private {{ |
| 348 * high: !Array<!Task>, | 360 * high: !Array<!Task>, |
| (...skipping 227 matching lines...) Expand 10 before | Expand all | Expand 10 after Loading... | |
| 576 function setSearchManagerForTesting(searchManager) { | 588 function setSearchManagerForTesting(searchManager) { |
| 577 SearchManagerImpl.instance_ = searchManager; | 589 SearchManagerImpl.instance_ = searchManager; |
| 578 } | 590 } |
| 579 | 591 |
| 580 return { | 592 return { |
| 581 getSearchManager: getSearchManager, | 593 getSearchManager: getSearchManager, |
| 582 setSearchManagerForTesting: setSearchManagerForTesting, | 594 setSearchManagerForTesting: setSearchManagerForTesting, |
| 583 SearchRequest: SearchRequest, | 595 SearchRequest: SearchRequest, |
| 584 }; | 596 }; |
| 585 }); | 597 }); |
| OLD | NEW |
