diff --git a/ui/file_manager/file_manager/foreground/css/file_manager.css b/ui/file_manager/file_manager/foreground/css/file_manager.css index 8ee8f7e0be8b3..c600fd315f446 100644 --- a/ui/file_manager/file_manager/foreground/css/file_manager.css +++ b/ui/file_manager/file_manager/foreground/css/file_manager.css @@ -1459,6 +1459,8 @@ body.files-ng grid .grid-title { font-family: 'Roboto Medium'; padding-inline-start: 16px; padding-top: 20px; + /* Make sure clicking this area can unselect file grids. */ + pointer-events: none; width: 100%; } @@ -1798,26 +1800,17 @@ body.files-ng #list-container li { } /* Modify GROUP_HEADING_HEIGHT in file_table_list.js if this height changes. */ -#list-container li[group-heading] { - /* The desired height plus the border width. */ - margin-top: calc(56px + 1px); -} - -#list-container li[group-heading]::before { +#list-container .group-heading { border-bottom: 1px solid var(--cros-separator-color); + box-sizing: border-box; color: var(--cros-text-color-secondary); - content: attr(group-heading); - display: block; font-family: 'Roboto Medium'; - left: 0; - padding-bottom: 16px; - padding-inline-end: 20px; + /* The desired height plus the border width. */ + height: calc(56px + 1px); padding-inline-start: 16px; padding-top: 20px; + /* Make sure clicking this area can unselect file items. */ pointer-events: none; - position: absolute; - /* Align with li[group-heading]'s margin-top. */ - top: -57px; width: 100%; } diff --git a/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn b/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn index 7fe090e1f108d..f4a71d1365bd0 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn +++ b/ui/file_manager/file_manager/foreground/js/ui/BUILD.gn @@ -399,18 +399,15 @@ js_library("file_table") { deps = [ ":a11y_announce", ":drag_selector", - ":file_list_selection_model", ":file_metadata_formatter", ":file_table_list", "table:table", "table:table_column", "table:table_column_model", - "table:table_list", "//ui/file_manager/file_manager/common/js:async_util", "//ui/file_manager/file_manager/common/js:file_type", "//ui/file_manager/file_manager/common/js:importer_common", "//ui/file_manager/file_manager/common/js:util", - "//ui/file_manager/file_manager/externs:entry_location", "//ui/file_manager/file_manager/externs:files_app_entry_interfaces", "//ui/file_manager/file_manager/externs:volume_manager", "//ui/file_manager/file_manager/externs/background:import_history", @@ -420,9 +417,6 @@ js_library("file_table") { "//ui/file_manager/file_manager/foreground/js/metadata:metadata_model", "//ui/webui/resources/js:assert.m", "//ui/webui/resources/js:cr.m", - "//ui/webui/resources/js/cr/ui:list.m", - "//ui/webui/resources/js/cr/ui:list_item.m", - "//ui/webui/resources/js/cr/ui:list_selection_model.m", ] } @@ -437,6 +431,7 @@ js_unittest("file_table_unittest") { js_library("file_table_list") { deps = [ ":a11y_announce", + ":drag_selector", ":file_list_selection_model", ":file_tap_handler", "table:table", diff --git a/ui/file_manager/file_manager/foreground/js/ui/file_table.js b/ui/file_manager/file_manager/foreground/js/ui/file_table.js index 483af670c6508..b55f4aaf5352f 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/file_table.js +++ b/ui/file_manager/file_manager/foreground/js/ui/file_table.js @@ -4,16 +4,12 @@ import {assert, assertInstanceof} from 'chrome://resources/js/assert.m.js'; import {dispatchSimpleEvent} from 'chrome://resources/js/cr.m.js'; -import {List} from 'chrome://resources/js/cr/ui/list.m.js'; -import {ListItem} from 'chrome://resources/js/cr/ui/list_item.m.js'; -import {ListSelectionModel} from 'chrome://resources/js/cr/ui/list_selection_model.m.js'; import {AsyncUtil} from '../../../common/js/async_util.js'; import {FileType} from '../../../common/js/file_type.js'; import {importer} from '../../../common/js/importer_common.js'; import {str, strf, util} from '../../../common/js/util.js'; import {importerHistoryInterfaces} from '../../../externs/background/import_history.js'; -import {EntryLocation} from '../../../externs/entry_location.js'; import {FilesAppEntry} from '../../../externs/files_app_entry_interfaces.js'; import {VolumeManager} from '../../../externs/volume_manager.js'; import {FilesTooltip} from '../../elements/files_tooltip.js'; @@ -23,13 +19,11 @@ import {MetadataModel} from '../metadata/metadata_model.js'; import {A11yAnnounce} from './a11y_announce.js'; import {DragSelector} from './drag_selector.js'; -import {FileListSelectionModel, FileListSingleSelectionModel} from './file_list_selection_model.js'; import {FileMetadataFormatter} from './file_metadata_formatter.js'; import {filelist, FileTableList} from './file_table_list.js'; import {Table} from './table/table.js'; import {TableColumn} from './table/table_column.js'; import {TableColumnModel} from './table/table_column_model.js'; -import {TableList} from './table/table_list.js'; /** * Custom column model for advanced auto-resizing. @@ -538,31 +532,6 @@ export class FileTable extends Table { }.bind(self), true); self.list.shouldStartDragSelection = self.shouldStartDragSelection_.bind(self); - self.list.hasDragHitElement = self.hasDragHitElement_.bind(self); - - /** - * Obtains the index list of elements that are hit by the point or the - * rectangle. - * - * @param {number} x X coordinate value. - * @param {number} y Y coordinate value. - * @param {number=} opt_width Width of the coordinate. - * @param {number=} opt_height Height of the coordinate. - * @return {Array} Index list of hit elements. - * @this {List} - */ - self.list.getHitElements = function(x, y, opt_width, opt_height) { - const currentSelection = []; - const bottom = y + (opt_height || 0); - for (let i = 0; i < this.selectionModel_.length; i++) { - const itemMetrics = this.getHeightsForIndex(i); - if (itemMetrics.top < bottom && - itemMetrics.top + itemMetrics.height >= y) { - currentSelection.push(i); - } - } - return currentSelection; - }; } /** @@ -758,19 +727,6 @@ export class FileTable extends Table { this.useModificationByMeTime_ = useModificationByMeTime; } - /** - * Returns whether the drag event is inside a file entry in the list (and not - * the background padding area). - * @param {MouseEvent} event Drag start event. - * @return {boolean} True if the mouse is over an element in the list, False - * if - * it is in the background. - */ - hasDragHitElement_(event) { - const pos = DragSelector.getScrolledPosition(this.list, event); - return this.list.getHitElements(pos.x, pos.y).length !== 0; - } - /** * Obtains if the drag selection should be start or not by referring the mouse * event. diff --git a/ui/file_manager/file_manager/foreground/js/ui/file_table_list.js b/ui/file_manager/file_manager/foreground/js/ui/file_table_list.js index a6c4f76171e80..2ecc0cc5dc8f7 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/file_table_list.js +++ b/ui/file_manager/file_manager/foreground/js/ui/file_table_list.js @@ -17,6 +17,7 @@ import {FileListModel} from '../file_list_model.js'; import {MetadataModel} from '../metadata/metadata_model.js'; import {A11yAnnounce} from './a11y_announce.js'; +import {DragSelector} from './drag_selector.js'; import {FileListSelectionModel, FileListSingleSelectionModel} from './file_list_selection_model.js'; import {FileTapHandler} from './file_tap_handler.js'; import {TableList} from './table/table_list.js'; @@ -26,7 +27,7 @@ import {TableList} from './table/table_list.js'; */ const filelist = {}; -// Group Heading height, align with CSS #list-container li[group-heading]. +// Group Heading height, align with CSS #list-container .group-heading. const GROUP_HEADING_HEIGHT = 57; /** @@ -86,9 +87,12 @@ export class FileTableList extends TableList { } // Check if index i is the start of a new group. if (startIndexToGroupLabel.has(i)) { - item.setAttribute('group-heading', startIndexToGroupLabel.get(i).label); - } else { - item.removeAttribute('group-heading'); + // For first item in each group, we add a title div before the element. + const title = document.createElement('div'); + title.innerText = startIndexToGroupLabel.get(i).label; + title.classList.add( + 'group-heading', `group-by-${fileListModel.groupByField}`); + this.insertBefore(title, item); } } @@ -198,6 +202,12 @@ export class FileTableList extends TableList { * Override here because previously we just need to use the index to multiply * the item height, now we also need to add up the potential group heading * heights included in these items. + * + * Note: for group start item, technically its height should be "all heights + * above it + current group heading height", but here we don't add the + * current group heading height (logic in getGroupHeadingCountBeforeIndex_), + * that's because it will break the "beforeFillerHeight" logic in the redraw + * of list.js. * @override */ getItemTop(index) { @@ -225,6 +235,53 @@ export class FileTableList extends TableList { return (this.dataModel.length - lastIndex) * itemHeight + countOfGroupHeadings * this.getGroupHeadingHeight_(); } + + /** + * Returns whether the drag event is inside a file entry in the list (and not + * the background padding area). + * @param {MouseEvent} event Drag start event. + * @return {boolean} True if the mouse is over an element in the list, False + * if it is in the background. + */ + hasDragHitElement(event) { + const pos = DragSelector.getScrolledPosition(this, event); + return this.getHitElements(pos.x, pos.y).length !== 0; + } + + /** + * Obtains the index list of elements that are hit by the point or the + * rectangle. + * + * @param {number} x X coordinate value. + * @param {number} y Y coordinate value. + * @param {number=} opt_width Width of the coordinate. + * @param {number=} opt_height Height of the coordinate. + * @return {Array} Index list of hit elements. + */ + getHitElements(x, y, opt_width, opt_height) { + const fileListModel = /** @type {FileListModel} */ (this.dataModel); + const groupBySnapshot = + fileListModel ? fileListModel.getGroupBySnapshot() : []; + const startIndexToGroupLabel = new Map(groupBySnapshot.map(group => { + return [group.startIndex, group]; + })); + + const currentSelection = []; + const startHeight = y; + const endHeight = y + (opt_height || 0); + for (let i = 0; i < this.selectionModel.length; i++) { + const itemMetrics = this.getHeightsForIndex(i); + // For group start item, we need to explicitly add group height because + // its top doesn't take that into consideration. (check notes in + // getItemTop()) + const itemTop = itemMetrics.top + + (startIndexToGroupLabel.has(i) ? this.getGroupHeadingHeight_() : 0); + if (itemTop < endHeight && itemTop + itemMetrics.height >= startHeight) { + currentSelection.push(i); + } + } + return currentSelection; + } } /** diff --git a/ui/file_manager/file_manager/foreground/js/ui/file_table_list_unittest.js b/ui/file_manager/file_manager/foreground/js/ui/file_table_list_unittest.js index aaa27d1e3a9bb..48271c96ae57e 100644 --- a/ui/file_manager/file_manager/foreground/js/ui/file_table_list_unittest.js +++ b/ui/file_manager/file_manager/foreground/js/ui/file_table_list_unittest.js @@ -3,7 +3,7 @@ // found in the LICENSE file. import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js'; -import {assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js'; +import {assertArrayEquals, assertEquals, assertFalse, assertTrue} from 'chrome://webui-test/chai_assert.js'; import {MockVolumeManager} from '../../../background/js/mock_volume_manager.js'; import {FakeEntryImpl} from '../../../common/js/files_app_entry_types.js'; @@ -535,3 +535,56 @@ export function testGetIndexForListOffset() { assertEquals(tableList.getIndexForListOffset_(400), 7); assertEquals(tableList.getIndexForListOffset_(500), 9); } + +export function testGetHitElements() { + const tableList = setupFileTableList(); + + // No group heading. + // index height total height + // ----------------------------------- + // Item 0 40 40 + // Item 1 40 80 + // Item 2 40 120 + // Item 3 40 160 + // Item 4 40 200 + // Item 5 40 240 + // Item 6 40 280 + // Item 7 40 320 + // Item 8 40 360 + // Item 9 40 400 + + // Passing -1 for 1st/3rd parameter because we don't care the x coordinates + // and the width of the drag selection. + assertArrayEquals(tableList.getHitElements(-1, 10), [0]); + assertArrayEquals( + tableList.getHitElements(-1, 50, -1, 200), [1, 2, 3, 4, 5, 6]); + assertArrayEquals(tableList.getHitElements(-1, 240, -1, 100), [5, 6, 7, 8]); + + // Enable group by. + enableGroupByForDataModel(tableList.dataModel); + // index height total height + // ----------------------------------- + // Heading 1 20 20 + // Item 0 40 60 + // Item 1 40 100 + // Heading 2 20 120 + // Item 2 40 160 + // Heading 3 20 180 + // Item 3 40 220 + // Item 4 40 260 + // Heading 4 20 280 + // Item 5 40 320 + // Item 6 40 360 + // Heading 5 20 380 + // Item 7 40 420 + // Item 8 40 460 + // Heading 6 20 480 + // Item 9 40 520 + + // Passing -1 for 1st/3rd parameter because we don't care the x coordinates + // and the width of the drag selection. + assertArrayEquals(tableList.getHitElements(-1, 10), []); + assertArrayEquals(tableList.getHitElements(-1, 40), [0]); + assertArrayEquals(tableList.getHitElements(-1, 50, -1, 200), [0, 1, 2, 3, 4]); + assertArrayEquals(tableList.getHitElements(-1, 220, -1, 100), [3, 4, 5]); +} diff --git a/ui/file_manager/integration_tests/file_manager/recents.js b/ui/file_manager/integration_tests/file_manager/recents.js index 6d389ebecd468..7dd5f85d1538d 100644 --- a/ui/file_manager/integration_tests/file_manager/recents.js +++ b/ui/file_manager/integration_tests/file_manager/recents.js @@ -1010,19 +1010,24 @@ testcase.recentsTimePeriodHeadings = async () => { await remoteCall.waitForFiles( appId, TestEntryInfo.getExpectedRows([todayFile, yesterdayFile]), { // Ignore last modified time because it will show Today/Yesterday - // instead - // of the actual date. + // instead of the actual date. ignoreLastModifiedTime: true, }); // Check headings in list view mode. - const todayListItem = - await remoteCall.waitForElement(appId, 'li[group-heading="Today"]'); + await remoteCall.waitForElementsCount(appId, ['.group-heading'], 2); + const groupHeadings = await remoteCall.callRemoteTestUtil( + 'deepQueryAllElements', appId, ['.group-heading']); + chrome.test.assertEq(2, groupHeadings.length); + const fileItems = await remoteCall.callRemoteTestUtil( + 'deepQueryAllElements', appId, ['.group-heading + .table-row']); + chrome.test.assertEq(2, fileItems.length); + + chrome.test.assertEq('Today', groupHeadings[0].text); chrome.test.assertEq( - todayFile.nameText, todayListItem.attributes['file-name']); - const yesterdayListItem = - await remoteCall.waitForElement(appId, 'li[group-heading="Yesterday"]'); + todayFile.nameText, fileItems[0].attributes['file-name']); + chrome.test.assertEq('Yesterday', groupHeadings[1].text); chrome.test.assertEq( - yesterdayFile.nameText, yesterdayListItem.attributes['file-name']); + yesterdayFile.nameText, fileItems[1].attributes['file-name']); // Switch to grid view. await remoteCall.waitAndClickElement(appId, '#view-button');