From 7f0b07fb489993528824492a08377d060ef837c5 Mon Sep 17 00:00:00 2001 From: Andrew Rayskiy Date: Fri, 25 Nov 2022 00:24:14 +0000 Subject: [PATCH] [FileManager] Annotate policy-assigned default apps with a managed icon. Bug: 1366815 Change-Id: I463df2c7ce2f37e6e37c8333c32ea50aa7acf0c1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4043923 Reviewed-by: Luciano Pacheco Commit-Queue: Andrew Rayskiy Cr-Commit-Position: refs/heads/main@{#1075651} --- .../file_manager/file_manager_browsertest.cc | 3 +- .../file_manager/background/js/test_util.js | 11 +++- .../file_manager/foreground/css/menu.css | 20 +++--- .../foreground/js/task_controller.ts | 15 ++++- .../foreground/js/ui/combobutton.js | 7 +- .../foreground/js/ui/files_menu.js | 39 ++++++++++- .../foreground/js/ui/menu_item.d.ts | 7 ++ ui/file_manager/file_names.gni | 1 + .../file_manager/context_menu.js | 64 ++++++++++++++++++- .../integration_tests/remote_call.js | 22 ++++++- 10 files changed, 167 insertions(+), 22 deletions(-) create mode 100644 ui/file_manager/file_manager/foreground/js/ui/menu_item.d.ts diff --git a/chrome/browser/ash/file_manager/file_manager_browsertest.cc b/chrome/browser/ash/file_manager/file_manager_browsertest.cc index e6f974e4fdb34..5fb5474daceae 100644 --- a/chrome/browser/ash/file_manager/file_manager_browsertest.cc +++ b/chrome/browser/ash/file_manager/file_manager_browsertest.cc @@ -978,7 +978,8 @@ WRAPPED_INSTANTIATE_TEST_SUITE_P( TestCase("checkDeleteDisabledInRecents"), TestCase("checkGoToFileLocationEnabledInRecents"), TestCase("checkGoToFileLocationDisabledInMultipleSelection"), - TestCase("checkDefaultTask"))); + TestCase("checkDefaultTask"), + TestCase("checkPolicyAssignedDefaultHasManagedIcon"))); WRAPPED_INSTANTIATE_TEST_SUITE_P( Toolbar, /* toolbar.js */ diff --git a/ui/file_manager/file_manager/background/js/test_util.js b/ui/file_manager/file_manager/background/js/test_util.js index 25f0ede7fc8c2..c51572f44ee29 100644 --- a/ui/file_manager/file_manager/background/js/test_util.js +++ b/ui/file_manager/file_manager/background/js/test_util.js @@ -383,13 +383,20 @@ test.util.sync.execCommand = (contentWindow, command) => { * @param {Window} contentWindow Window to be tested. * @param {Array} taskList List of tasks to be returned in * fileManagerPrivate.getFileTasks(). + * @param {boolean} + * isPolicyDefault Whether the default is set by policy. * @return {boolean} Always return true. */ -test.util.sync.overrideTasks = (contentWindow, taskList) => { +test.util.sync + .overrideTasks = (contentWindow, taskList, isPolicyDefault = false) => { const getFileTasks = (entries, onTasks) => { // Call onTask asynchronously (same with original getFileTasks). setTimeout(() => { - onTasks({tasks: taskList}); + const policyDefaultHandlerStatus = isPolicyDefault ? + chrome.fileManagerPrivate.PolicyDefaultHandlerStatus + .DEFAULT_HANDLER_ASSIGNED_BY_POLICY : + undefined; + onTasks({tasks: taskList, policyDefaultHandlerStatus}); }, 0); }; diff --git a/ui/file_manager/file_manager/foreground/css/menu.css b/ui/file_manager/file_manager/foreground/css/menu.css index b8888772d7899..aebbe5f6dcf52 100644 --- a/ui/file_manager/file_manager/foreground/css/menu.css +++ b/ui/file_manager/file_manager/foreground/css/menu.css @@ -73,7 +73,9 @@ cr-menu[hidden].files-menu.animating { } cr-menu.files-menu > cr-menu-item { - position: relative; + display: flex; + align-items: center; + flex-direction: row; } /* Icon on the left of the item label for cr.ui.FilesMenuItem. @@ -91,7 +93,7 @@ cr-menu.files-menu cr-menu-item .icon { } cr-menu.files-menu cr-menu-item .icon.start { - display: inline-block; + align-self: flex-start; margin-inline-end: 8px; } @@ -99,22 +101,22 @@ cr-menu.files-menu:not(.has-icon-start) cr-menu-item .icon.start { display: none; } -cr-menu.files-menu cr-menu-item .icon.end { - float: right; +cr-menu.files-menu cr-menu-item .icon.managed { + background-image: url(chrome://resources/images/business.svg); + margin-inline-start: 8px; } -html[dir='rtl'] cr-menu.files-menu cr-menu-item .icon.end { - float: left; +cr-menu.files-menu cr-menu-item .icon.end { + align-self: flex-end; + margin-inline-start: 8px; } cr-menu.files-menu > cr-menu-item > .icon { - position: relative; z-index: 1; } cr-menu.files-menu > cr-menu-item > span { - position: relative; - vertical-align: middle; + flex-grow: 1; z-index: 1; } diff --git a/ui/file_manager/file_manager/foreground/js/task_controller.ts b/ui/file_manager/file_manager/foreground/js/task_controller.ts index 1cad4887f7469..f678a10a3af7a 100644 --- a/ui/file_manager/file_manager/foreground/js/task_controller.ts +++ b/ui/file_manager/file_manager/foreground/js/task_controller.ts @@ -238,9 +238,12 @@ export class TaskController { // an item to change default task. if (defaultTask) { combobutton.addSeparator(); + // TODO(greengrape): Ensure that the passed object is a `DropdownItem`. const changeDefaultMenuItem = combobutton.addDropDownItem({ type: TaskMenuItemType.CHANGE_DEFAULT_TASK, label: str('CHANGE_DEFAULT_MENU_ITEM'), + isDefault: false, + isPolicyDefault: false, }); changeDefaultMenuItem.classList.add('change-default'); @@ -272,7 +275,9 @@ export class TaskController { for (const task of tasks) { if (task === fileTasks.defaultTask) { const title = task.title + ' ' + str('DEFAULT_TASK_LABEL'); - items.push(createDropdownItem(task, title, true, true)); + items.push(createDropdownItem( + task, title, /*bold=*/ true, /*isDefault=*/ true, + /*isPolicyDefault=*/ !!fileTasks.getPolicyDefaultHandlerStatus())); } else { items.push(createDropdownItem(task)); } @@ -480,6 +485,7 @@ export class TaskController { if (taskCount > 0) { if (defaultTask) { const menuItem = this.ui_.defaultTaskMenuItem; + menuItem.setIsDefaultAttribute(); /** * Menu icon can be controlled by either `iconEndImage` or * `iconEndFileType`, since the default task menu item DOM is shared, @@ -500,6 +506,9 @@ export class TaskController { menuItem.setIconEndHidden(true); } + menuItem.toggleManagedIcon( + /*visible=*/ !!openTasks.policyDefaultHandlerStatus); + menuItem.label = defaultTask.title; menuItem.descriptor = defaultTask.descriptor; } @@ -656,6 +665,7 @@ export interface DropdownItem { task: chrome.fileManagerPrivate.FileTask; bold: boolean; isDefault: boolean; + isPolicyDefault: boolean; isGenericFileHandler?: boolean; } @@ -666,7 +676,7 @@ export interface DropdownItem { */ function createDropdownItem( task: chrome.fileManagerPrivate.FileTask, title?: string, bold?: boolean, - isDefault?: boolean): DropdownItem { + isDefault?: boolean, isPolicyDefault?: boolean): DropdownItem { return { type: TaskMenuItemType.RUN_TASK, label: title || task.title, @@ -675,6 +685,7 @@ function createDropdownItem( task: task, bold: bold || false, isDefault: isDefault || false, + isPolicyDefault: isPolicyDefault || false, isGenericFileHandler: task.isGenericFileHandler, }; } diff --git a/ui/file_manager/file_manager/foreground/js/ui/combobutton.js b/ui/file_manager/file_manager/foreground/js/ui/combobutton.js index 59a65ab37bbc2..77c44c8b06b5a 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/combobutton.js +++ b/ui/file_manager/file_manager/foreground/js/ui/combobutton.js @@ -47,11 +47,16 @@ export class ComboButton extends MultiMenuButton { addDropDownItem(item) { this.multiple = true; - const menuitem = this.menu.addMenuItem(item); + const menuitem = /** @type {!MenuItem} */ (this.menu.addMenuItem(item)); // If menu is files-menu, decorate menu item as FilesMenuItem. if (this.menu.classList.contains('files-menu')) { decorate(menuitem, FilesMenuItem); + /** @type {!FilesMenuItem} */ (menuitem).toggleManagedIcon( + /*visible=*/ item.isPolicyDefault); + if (item.isDefault) { + /** @type {!FilesMenuItem} */ (menuitem).setIsDefaultAttribute(); + } } menuitem.data = item; diff --git a/ui/file_manager/file_manager/foreground/js/ui/files_menu.js b/ui/file_manager/file_manager/foreground/js/ui/files_menu.js index 8935779d29a15..e81534d2d77d0 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/files_menu.js +++ b/ui/file_manager/file_manager/foreground/js/ui/files_menu.js @@ -25,6 +25,9 @@ export class FilesMenuItem extends MenuItem { /** @private {?HTMLElement} */ this.iconStart_ = null; + /** @private {?HTMLElement} */ + this.iconManaged_ = null; + /** @private {?HTMLElement} */ this.iconEnd_ = null; @@ -64,20 +67,26 @@ export class FilesMenuItem extends MenuItem { assertInstanceof(document.createElement('div'), HTMLElement); this.iconStart_.classList.add('icon', 'start'); + this.iconManaged_ = + assertInstanceof(document.createElement('div'), HTMLElement); + this.iconManaged_.classList.add('icon', 'managed'); + this.iconEnd_ = assertInstanceof(document.createElement('div'), HTMLElement); this.iconEnd_.classList.add('icon', 'end'); /** - * This is hidden by default because most of the menu items don't require - * an end icon, the component which uses end icon should explicitly make - * it visible. + * This is hidden by default because most of the menu items require + * neither the end icon nor the managed icon, so the component that + * plans to use either end icon should explicitly make it visible. */ this.setIconEndHidden(true); + this.toggleManagedIcon(/*visible=*/ false); // Override with standard menu item elements. this.textContent = ''; this.appendChild(this.iconStart_); this.appendChild(this.label_); + this.appendChild(this.iconManaged_); this.appendChild(this.iconEnd_); } @@ -237,6 +246,30 @@ export class FilesMenuItem extends MenuItem { this.iconStart_.setAttribute('file-type-icon', value); } + /** + * Sets or removes the `is-managed` attribute. + * @param {boolean} isManaged + */ + toggleIsManagedAttribute(isManaged) { + this.toggleAttribute('is-managed', isManaged); + } + + /** + * Sets the `is-default` attribute. + */ + setIsDefaultAttribute() { + this.toggleAttribute('is-default', true); + } + + /** + * Toggles visibility of the `Managed by Policy` icon. + * @param {boolean} visible + */ + toggleManagedIcon(visible) { + this.iconManaged_.toggleAttribute('hidden', !visible); + this.toggleIsManagedAttribute(visible); + } + /** * @return {string} */ diff --git a/ui/file_manager/file_manager/foreground/js/ui/menu_item.d.ts b/ui/file_manager/file_manager/foreground/js/ui/menu_item.d.ts new file mode 100644 index 0000000000000..054beaf292d55 --- /dev/null +++ b/ui/file_manager/file_manager/foreground/js/ui/menu_item.d.ts @@ -0,0 +1,7 @@ +// Copyright 2022 The Chromium Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +export class MenuItem extends HTMLElement { + disabled: boolean; +} diff --git a/ui/file_manager/file_names.gni b/ui/file_manager/file_names.gni index f1db801993e21..3aaabbe1e00db 100644 --- a/ui/file_manager/file_names.gni +++ b/ui/file_manager/file_names.gni @@ -346,6 +346,7 @@ checked_in_dts_files = [ "file_manager/foreground/js/ui/command.d.ts", "file_manager/foreground/js/ui/list.d.ts", "file_manager/foreground/js/ui/list_item.d.ts", + "file_manager/foreground/js/ui/menu_item.d.ts", "file_manager/foreground/js/ui/multi_menu_button.d.ts", ] diff --git a/ui/file_manager/integration_tests/file_manager/context_menu.js b/ui/file_manager/integration_tests/file_manager/context_menu.js index d003c062119b6..79168bea7fe23 100644 --- a/ui/file_manager/integration_tests/file_manager/context_menu.js +++ b/ui/file_manager/integration_tests/file_manager/context_menu.js @@ -882,7 +882,6 @@ testcase.checkContextMenuFocus = async () => { chrome.test.assertEq('menuitem', focusedElement.attributes['role']); }; - testcase.checkDefaultTask = async () => { // Open FilesApp on Downloads. const appId = await setupAndWaitUntilReady( @@ -925,3 +924,66 @@ testcase.checkDefaultTask = async () => { chrome.test.assertTrue(!!folderDefaultTaskItem); chrome.test.assertTrue(folderDefaultTaskItem.hidden); }; + +testcase.checkPolicyAssignedDefaultHasManagedIcon = async () => { + // Open FilesApp on Downloads. + const appId = + await setupAndWaitUntilReady(RootPath.DOWNLOADS, [ENTRIES.hello], []); + + // Force a task for the `hello` file. + const fakeDefaultTask = new FakeTask( + /* isDefault */ true, + {appId: 'dummyId1', taskType: 'app', actionId: 'open-with'}, + 'DummyDefaultTask'); + const fakeSecondaryTask = new FakeTask( + /* isDefault */ false, + {appId: 'dummyId2', taskType: 'app', actionId: 'open-with'}, + 'DummySecondaryTask'); + + await remoteCall.callRemoteTestUtil( + 'overrideTasks', appId, + [[fakeDefaultTask, fakeSecondaryTask], /*isPolicyDefault=*/ true]); + + // Display the context menu. + await remoteCall.showContextMenuFor(appId, ENTRIES.hello.nameText); + + // Get the context menu. + const contextMenu = await remoteCall.getMenu(appId, 'context-menu'); + + // Check the default task item is visible and has is-default/is-managed + // properties set. + const contextMenuDefaultTaskItem = contextMenu['items'].find( + el => el.attributes.id === 'default-task-menu-item'); + chrome.test.assertTrue(!!contextMenuDefaultTaskItem); + chrome.test.assertFalse(contextMenuDefaultTaskItem.hidden); + chrome.test.assertEq(contextMenuDefaultTaskItem.text, 'DummyDefaultTask'); + chrome.test.assertTrue('is-default' in contextMenuDefaultTaskItem.attributes); + chrome.test.assertTrue('is-managed' in contextMenuDefaultTaskItem.attributes); + + // Dismiss the context menu. + await remoteCall.dismissMenu(appId); + + // Display the tasks menu. + await remoteCall.expandOpenDropdown(appId); + + // Get the tasks menu. + const tasksMenu = await remoteCall.getMenu(appId, 'tasks'); + + // Check the default task item is visible and has is-default/is-managed + // properties set. + const tasksMenuDefaultTaskItem = tasksMenu['items'][0]; + chrome.test.assertTrue(!!tasksMenuDefaultTaskItem); + chrome.test.assertFalse(tasksMenuDefaultTaskItem.hidden); + chrome.test.assertTrue( + tasksMenuDefaultTaskItem.text.includes('DummyDefaultTask')); + chrome.test.assertTrue('is-default' in tasksMenuDefaultTaskItem.attributes); + chrome.test.assertTrue('is-managed' in tasksMenuDefaultTaskItem.attributes); + + // Check that the remaining items do not have is-default/is-managed + // properties. + const tasksMenuNonDefaultTaskItems = tasksMenu['items'].slice(1); + for (const nonDefaultTaskItem of tasksMenuNonDefaultTaskItems) { + chrome.test.assertFalse('is-default' in nonDefaultTaskItem.attributes); + chrome.test.assertFalse('is-managed' in nonDefaultTaskItem.attributes); + } +}; diff --git a/ui/file_manager/integration_tests/remote_call.js b/ui/file_manager/integration_tests/remote_call.js index eeefdd2881e4e..13e9729b1de90 100644 --- a/ui/file_manager/integration_tests/remote_call.js +++ b/ui/file_manager/integration_tests/remote_call.js @@ -932,19 +932,22 @@ export class RemoteCallFilesApp extends RemoteCall { /** * @param {string} appId App window Id. * @param {string|!Array} query Query to find the elements. - * @return {!Promise} Promise to be fulfilled with the + * @return {!Promise>} Promise to be fulfilled with the * elements. * @private */ async queryElements_(appId, query) { - return this.callRemoteTestUtil('deepQueryAllElements', appId, [query]); + if (typeof query === 'string') { + query = [query]; + } + return this.callRemoteTestUtil('deepQueryAllElements', appId, query); } /** * Returns the menu as ElementObject and its menu-items (including separators) * in the `items` property. * @param {string} appId App window Id. - * @param {string|!Array} menu Query to find the elements. + * @param {string|!Array} menu The name of the menu. * @return {!Promise} Promise to be fulfilled with * the menu. */ @@ -953,7 +956,10 @@ export class RemoteCallFilesApp extends RemoteCall { // TODO: Implement for other menus. if (menu === 'context-menu') { menuId = '#file-context-menu'; + } else if (menu == 'tasks') { + menuId = '#tasks-menu'; } + if (!menuId) { console.error(`Invalid menu '${menu}'`); return; @@ -965,4 +971,14 @@ export class RemoteCallFilesApp extends RemoteCall { menuElement.items = await this.queryElements_(appId, `${menuId} > *`); return menuElement; } + + /** + * Displays the "tasks" menu from the "OPEN" button dropdown. + * The caller code has to prepare the selection to have multiple tasks. + * @param {string} appId App window Id. + */ + async expandOpenDropdown(appId) { + // Wait the OPEN button to have multiple tasks. + await this.waitAndClickElement(appId, '#tasks[multiple]'); + } }