Commit 550669ad authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Files app: Fix breadcrumbs to have tooltip

Add tooltip and aria-label to breadcrumbs. This fixes 3 issues:
1. Make the collapsed breadcrumb accessible to Chromevox issue:1029245
2. Show tooltip for breadcrumbs useful for:
 2.1 When hovering a collapsed breadcrumb, which displays "..." as
     content.
 2.2 When the name is too long and part of the content shows as
     ellipses.

Change variable to const where enforced by presubmit.

Change param from HTMLElement to EventTarget, because it only uses the
addEventListener() and closure failed in the new call to addTarget().

Fix test filesTooltipFocus() which expected breadcrumb to not have
tooltip, but setting the focus to file list instead.

Test: browser_tests --gtest_filter="*breadcrumbsTooltip"
Bug: 1029245, 951305
Change-Id: I74ba38519ac70b1e210f55e1979a862841b13b1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1941533
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Reviewed-by: default avatarAlex Danilo <adanilo@chromium.org>
Auto-Submit: Luciano Pacheco <lucmult@chromium.org>
Cr-Commit-Position: refs/heads/master@{#720089}
parent 6a4e5961
......@@ -846,6 +846,7 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P(
FilesAppBrowserTest,
::testing::Values(TestCase("breadcrumbsNavigate"),
TestCase("breadcrumbsLeafNoFocus"),
TestCase("breadcrumbsTooltip"),
TestCase("breadcrumbsDownloadsTranslation")));
WRAPPED_INSTANTIATE_TEST_SUITE_P(
......
......@@ -12,7 +12,7 @@
* document.querySelector('files-tooltip').addTargets(
* document.querySelectorAll('[has-tooltip]'))
*/
var FilesTooltip = Polymer({
const FilesTooltip = Polymer({
is: 'files-tooltip',
properties: {
......@@ -80,7 +80,7 @@ var FilesTooltip = Polymer({
/**
* Adds a target to tooltip.
* @param {!HTMLElement} target
* @param {!EventTarget} target
*/
addTarget: function(target) {
target.addEventListener('mouseover', this.onMouseOver_.bind(this, target));
......
......@@ -490,8 +490,11 @@ class FileManagerUI {
* Attaches files tooltip.
*/
attachFilesTooltip() {
assertInstanceof(document.querySelector('files-tooltip'), FilesTooltip)
.addTargets(document.querySelectorAll('[has-tooltip]'));
const filesTooltip =
assertInstanceof(document.querySelector('files-tooltip'), FilesTooltip);
filesTooltip.addTargets(document.querySelectorAll('[has-tooltip]'));
this.locationLine.filesTooltip = filesTooltip;
}
/**
......
......@@ -20,6 +20,19 @@ class LocationLine extends cr.EventTarget {
this.listContainer_ = listContainer;
this.entry_ = null;
this.components_ = [];
/** @private {?FilesTooltip} */
this.filesTooltip_ = null;
}
/**
* @param {?FilesTooltip} filesTooltip
* */
set filesTooltip(filesTooltip) {
this.filesTooltip_ = filesTooltip;
this.filesTooltip_.addTargets(
this.breadcrumbs_.querySelectorAll('[has-tooltip]'));
}
/**
......@@ -184,10 +197,17 @@ class LocationLine extends cr.EventTarget {
button.id = 'breadcrumb-path-' + i;
button.classList.add(
'breadcrumb-path', 'entry-name', 'imitate-paper-button');
button.setAttribute('aria-label', component.name);
button.setAttribute('has-tooltip', '');
if (this.filesTooltip_) {
this.filesTooltip_.addTarget(button);
}
const nameElement = document.createElement('div');
nameElement.classList.add('name');
nameElement.textContent = component.name;
button.appendChild(nameElement);
button.addEventListener('click', this.onClick_.bind(this, i));
newBreadcrumbs.appendChild(button);
......
......@@ -72,7 +72,7 @@
// Check the breadcrumbs for Downloads:
// Os meu ficheiros => My files.
// Transferências => Downloads (as in Transfers).
let path =
const path =
await remoteCall.callRemoteTestUtil('getBreadcrumbPath', appId, []);
chrome.test.assertEq('/Os meus ficheiros/Transferências', path);
......@@ -84,4 +84,63 @@
await remoteCall.waitUntilCurrentDirectoryIsChanged(
appId, '/Os meus ficheiros/Transferências/photos');
};
/**
* Tests that breadcrumbs has to tooltip. crbug.com/951305
*/
testcase.breadcrumbsTooltip = async () => {
/**
* Helper to create a test folder from a full |path|.
* @param {string} path The folder path.
* @return {TestEntryInfo}
*/
function createTestFolder(path) {
const name = path.split('/').pop();
return new TestEntryInfo({
targetPath: path,
nameText: name,
type: EntryType.DIRECTORY,
lastModifiedTime: 'Jan 1, 1980, 11:59 PM',
sizeText: '--',
typeText: 'Folder',
});
}
// Build an array of nested folder test entries.
const nestedFolderTestEntries = [];
for (let path = 'nested-folder0', i = 0; i < 8; ++i) {
nestedFolderTestEntries.push(createTestFolder(path));
path += `/nested-folder${i + 1}`;
}
// Open FilesApp on Downloads containing the test entries.
const appId = await setupAndWaitUntilReady(
RootPath.DOWNLOADS, nestedFolderTestEntries, []);
// Navigate to deepest folder.
const breadcrumb = '/My files/Downloads/' +
nestedFolderTestEntries.map(e => e.nameText).join('/');
await navigateWithDirectoryTree(appId, breadcrumb);
// Get breadcrumb that is "collapsed" in the default window size.
const collapsedBreadcrumb =
await remoteCall.waitForElement(appId, '#breadcrumb-path-8');
// Check: Should have aria-label so screenreader announces the folder name.
chrome.test.assertEq(
'nested-folder6', collapsedBreadcrumb.attributes['aria-label']);
// Check: Should have collapsed attribute so it shows as "...".
chrome.test.assertEq('', collapsedBreadcrumb.attributes['collapsed']);
// Focus the search box.
chrome.test.assertTrue(await remoteCall.callRemoteTestUtil(
'fakeEvent', appId, ['#breadcrumb-path-8', 'focus']));
// Check: The tooltip should be visible and its content is the folder name.
const tooltipQueryVisible = 'files-tooltip[visible=true]';
const tooltip = await remoteCall.waitForElement(appId, tooltipQueryVisible);
const label =
await remoteCall.waitForElement(appId, [tooltipQueryVisible, '#label']);
chrome.test.assertEq('nested-folder6', label.text);
};
})();
......@@ -9,7 +9,7 @@
const tooltipQueryVisible = 'files-tooltip[visible=true]';
const searchButton = '#search-button[has-tooltip]';
const viewButton = '#view-button[has-tooltip]';
const breadcrumbs = '#breadcrumb-path-0';
const fileList = '#file-list';
const cancelButton = '#cancel-selection-button[has-tooltip]';
/**
......@@ -40,9 +40,9 @@
await remoteCall.waitForElement(appId, [tooltipQueryVisible, '#label']);
chrome.test.assertEq('Switch to thumbnail view', label.text);
// Focus a button without tooltip.
// Focus an element without tooltip.
chrome.test.assertTrue(
await remoteCall.callRemoteTestUtil('focus', appId, [breadcrumbs]));
await remoteCall.callRemoteTestUtil('focus', appId, [fileList]));
// The tooltip should be hidden.
tooltip = await remoteCall.waitForElement(appId, tooltipQueryHidden);
......@@ -112,7 +112,7 @@
// The tooltip should be visible.
tooltip = await remoteCall.waitForElement(appId, tooltipQueryVisible);
let label =
const label =
await remoteCall.waitForElement(appId, [tooltipQueryVisible, '#label']);
chrome.test.assertEq('Search', label.text);
......
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