Commit b7c838b5 authored by Alex Danilo's avatar Alex Danilo Committed by Commit Bot

Delay hiding search box until mouse click

Selecting a file, followed by opening the search box and attempting to
click 'delete' or 'open' buttons to the left of the search box failed
due to a mousedown listener that removed the search box and in turn,
moved the position of the 'delete' or 'open' buttons mid-click causing
the user action to fail.

Adds a new class to the search box 'hide-pending' that defers the
shrinking of the search box until a 'click' event is received on any
part of the action bar.

Noted during debugging that aria-disabled wasn't being set correctly, so
replace the use of 'hidden' for the <cr-input> element with 'disabled'
to control show/hide of the input text area, that updates aria-hidden
true || false correctly in the <cr-input>.

Change the Ctrl-F/Escape open and close of the search box to set the
'disabled' property instead of 'hidden'.

Bug: 668427
Tests: browser_tests --gtest_filter="*HidingTextEntryField"
Change-Id: Iac5f1cc0bec31f024f397797dd9c0397a6d703e6
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940325
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Reviewed-by: default avatarNoel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720096}
parent ab5f5c91
...@@ -835,7 +835,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P( ...@@ -835,7 +835,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
::testing::Values(TestCase("searchDownloadsWithResults"), ::testing::Values(TestCase("searchDownloadsWithResults"),
TestCase("searchDownloadsWithNoResults"), TestCase("searchDownloadsWithNoResults"),
TestCase("searchDownloadsClearSearchKeyDown"), TestCase("searchDownloadsClearSearchKeyDown"),
TestCase("searchDownloadsClearSearch"))); TestCase("searchDownloadsClearSearch"),
TestCase("searchHidingTextEntryField")));
WRAPPED_INSTANTIATE_TEST_SUITE_P( WRAPPED_INSTANTIATE_TEST_SUITE_P(
Metrics, /* metrics.js */ Metrics, /* metrics.js */
......
...@@ -690,12 +690,14 @@ body.check-select #search-box { ...@@ -690,12 +690,14 @@ body.check-select #search-box {
} }
#search-box.has-cursor, #search-box.has-cursor,
#search-box.has-text { #search-box.has-text,
#search-box.hide-pending {
margin-inline-end: 12px; margin-inline-end: 12px;
} }
#search-box.has-cursor cr-input, #search-box.has-cursor cr-input,
#search-box.has-text cr-input { #search-box.has-text cr-input,
#search-box.hide-pending cr-input {
width: 202px; width: 202px;
} }
......
...@@ -1517,7 +1517,7 @@ CommandHandler.COMMANDS_['search'] = new class extends Command { ...@@ -1517,7 +1517,7 @@ CommandHandler.COMMANDS_['search'] = new class extends Command {
// Focus and unhide the search box. // Focus and unhide the search box.
const element = fileManager.document.querySelector('#search-box cr-input'); const element = fileManager.document.querySelector('#search-box cr-input');
element.hidden = false; element.disabled = false;
(/** @type {!CrInputElement} */ (element)).select(); (/** @type {!CrInputElement} */ (element)).select();
} }
......
...@@ -169,6 +169,10 @@ class FileManagerUI { ...@@ -169,6 +169,10 @@ class FileManagerUI {
this.searchBox = new SearchBox( this.searchBox = new SearchBox(
queryRequiredElement('#search-box', this.element), queryRequiredElement('#search-box', this.element),
queryRequiredElement('#search-button', this.element)); queryRequiredElement('#search-button', this.element));
// Add a listener to the containing action bar for hiding the search box.
this.actionbar.addEventListener('click', (event) => {
this.searchBox.removeHidePending(event);
});
/** /**
* Empty folder UI. * Empty folder UI.
......
...@@ -102,10 +102,15 @@ class SearchBox extends cr.EventTarget { ...@@ -102,10 +102,15 @@ class SearchBox extends cr.EventTarget {
} }
/** /**
* Handles a focus event of the search box. * Handles a focus event of the search box <cr-input> element.
* @private * @private
*/ */
onFocus_() { onFocus_() {
// Early out if we closing the search cr-input: do not just go ahead and
// re-open it on focus, crbug.com/668427.
if (this.element.classList.contains('hide-pending')) {
return;
}
this.element.classList.toggle('has-cursor', true); this.element.classList.toggle('has-cursor', true);
this.autocompleteList.attachToInput(this.inputElement); this.autocompleteList.attachToInput(this.inputElement);
this.updateStyles_(); this.updateStyles_();
...@@ -114,16 +119,33 @@ class SearchBox extends cr.EventTarget { ...@@ -114,16 +119,33 @@ class SearchBox extends cr.EventTarget {
} }
/** /**
* Handles a blur event of the search box. * Handles a blur event of the search box <cr-input> element.
* @private * @private
*/ */
onBlur_() { onBlur_() {
this.element.classList.toggle('has-cursor', false); this.element.classList.toggle('has-cursor', false);
this.element.classList.toggle('hide-pending', true);
this.autocompleteList.detach(); this.autocompleteList.detach();
this.updateStyles_(); this.updateStyles_();
this.searchButtonToggleRipple_.activated = false; this.searchButtonToggleRipple_.activated = false;
// When input has any text we keep it displayed with current search. }
this.inputElement.hidden = this.inputElement.value.length == 0;
/**
* Handles delayed hiding of the search box (until click).
* @param {Event} event
*/
removeHidePending(event) {
if (this.element.classList.contains('hide-pending')) {
// If the search box was waiting to hide, but we clicked on it, don't.
if (event.target === this.inputElement) {
this.element.classList.toggle('hide-pending', false);
this.onFocus_();
} else {
// When input has any text we keep it displayed with current search.
this.inputElement.disabled = this.inputElement.value.length == 0;
this.element.classList.toggle('hide-pending', false);
}
}
} }
/** /**
...@@ -140,6 +162,8 @@ class SearchBox extends cr.EventTarget { ...@@ -140,6 +162,8 @@ class SearchBox extends cr.EventTarget {
this.inputElement.tabIndex = -1; // Focus to default element after blur. this.inputElement.tabIndex = -1; // Focus to default element after blur.
this.inputElement.blur(); this.inputElement.blur();
this.inputElement.disabled = this.inputElement.value.length == 0;
this.element.classList.toggle('hide-pending', false);
} }
/** /**
...@@ -183,7 +207,7 @@ class SearchBox extends cr.EventTarget { ...@@ -183,7 +207,7 @@ class SearchBox extends cr.EventTarget {
* @private * @private
*/ */
onSearchButtonClick_() { onSearchButtonClick_() {
this.inputElement.hidden = false; this.inputElement.disabled = false;
this.inputElement.focus(); this.inputElement.focus();
} }
......
...@@ -393,7 +393,7 @@ ...@@ -393,7 +393,7 @@
<div class="icon"></div> <div class="icon"></div>
</button> </button>
<div id="search-box"> <div id="search-box">
<cr-input type="search" tabindex="14" hidden <cr-input type="search" tabindex="14" disabled
aria-label="$i18n{SEARCH_TEXT_LABEL}" placeholder="$i18n{SEARCH_TEXT_LABEL}"> aria-label="$i18n{SEARCH_TEXT_LABEL}" placeholder="$i18n{SEARCH_TEXT_LABEL}">
<cr-button class="clear" slot="suffix" tabindex="14" <cr-button class="clear" slot="suffix" tabindex="14"
aria-label="$i18n{SEARCH_CLEAR_LABEL}" has-tooltip></cr-button> aria-label="$i18n{SEARCH_CLEAR_LABEL}" has-tooltip></cr-button>
......
...@@ -121,4 +121,46 @@ ...@@ -121,4 +121,46 @@
await remoteCall.waitForElement(appId, '#search-box [type="search"]'); await remoteCall.waitForElement(appId, '#search-box [type="search"]');
chrome.test.assertEq('', searchInput.value); chrome.test.assertEq('', searchInput.value);
}; };
/**
* Tests that the search text entry box stays expanded until the end of user
* interaction.
*/
testcase.searchHidingTextEntryField = async () => {
const entry = ENTRIES.hello;
// Open Files app on Downloads.
const appId = await setupAndWaitUntilReady(RootPath.DOWNLOADS, [entry], []);
// Select an entry in the file list.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'selectFile', appId, [entry.nameText]));
// Focus the toolbar search button.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'fakeEvent', appId, ['#search-button', 'click']));
// Verify the toolbar search text entry box is enabled.
let textInputElement =
await remoteCall.waitForElement(appId, ['#search-box cr-input']);
chrome.test.assertEq('false', textInputElement.attributes['aria-disabled']);
// Send a 'mousedown' to the toolbar 'delete' button.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'fakeEvent', appId, ['#delete-button', 'mousedown']));
// Verify the toolbar search text entry is still enabled.
textInputElement =
await remoteCall.waitForElement(appId, ['#search-box cr-input']);
chrome.test.assertEq('false', textInputElement.attributes['aria-disabled']);
// Send a 'mouseup' to the toolbar 'delete' button.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'fakeEvent', appId, ['#delete-button', 'mouseup']));
// Verify the toolbar search text entry is still enabled.
textInputElement =
await remoteCall.waitForElement(appId, ['#search-box cr-input']);
chrome.test.assertEq('false', textInputElement.attributes['aria-disabled']);
};
})(); })();
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment