Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(storage-browser): display paths on subfolder search #6193

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Prev Previous commit
Next Next commit
test: update unit tests
  • Loading branch information
pranavosu committed Nov 21, 2024
commit 661335f8546621f0d3a81d7e0dcc8661394819c8
Original file line number Diff line number Diff line change
Expand Up @@ -38,17 +38,13 @@ export interface LocationData {

export interface FolderData {
key: string;
path?: string;
name?: string;
id: string;
type: 'FOLDER';
}

export interface FileData {
eTag?: string;
key: string;
path?: string;
name?: string;
lastModified: Date;
id: string;
size: number;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ export const getFilteredLocations = (
export const getFileKey = (key: string): string =>
key.slice(key.lastIndexOf('/') + 1, key.length);

export const getFilePath = (key: string): string =>
key.slice(0, key.lastIndexOf('/') + 1);

export const getFolderPath = (key: string): string =>
getFilePath(key.slice(0, -1));

export const getFolderName = (key: string): string =>
getFileKey(key.slice(0, -1));

export const createFileDataItem = (data: FileData): FileDataItem => ({
...data,
fileKey: getFileKey(data.key),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,6 @@ export function searchItems<T>({ prefix = '', list, options }: Search<T>): T[] {
...item,
id: crypto.randomUUID(),
[key]: isFolder ? matchedPath + groupBy : matchedPath,
name: isFolder ? component + groupBy : component,
path: matchedPath.slice(prefix.length).slice(0, -component.length),
type: isFolder ? 'FOLDER' : 'FILE',
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ exports[`getActionViewTableData should handle tasks with prefix keys 1`] = `
},
{
"content": {
"text": "/",
"text": "folder/subfolder/",
},
"key": "folder-1",
"type": "text",
Expand Down Expand Up @@ -66,7 +66,7 @@ exports[`getActionViewTableData should handle tasks with prefix keys 1`] = `
},
{
"content": {
"text": "/",
"text": "/root/",
},
"key": "folder-2",
"type": "text",
Expand Down Expand Up @@ -180,7 +180,7 @@ exports[`getActionViewTableData should return correct table data for all task st
},
{
"content": {
"text": "/",
"text": "some-prefix/",
},
"key": "folder-1",
"type": "text",
Expand Down Expand Up @@ -232,7 +232,7 @@ exports[`getActionViewTableData should return correct table data for all task st
},
{
"content": {
"text": "/",
"text": "some-prefix/",
},
"key": "folder-2",
"type": "text",
Expand Down Expand Up @@ -284,7 +284,7 @@ exports[`getActionViewTableData should return correct table data for all task st
},
{
"content": {
"text": "/",
"text": "some-prefix/",
},
"key": "folder-3",
"type": "text",
Expand Down Expand Up @@ -336,7 +336,7 @@ exports[`getActionViewTableData should return correct table data for all task st
},
{
"content": {
"text": "/",
"text": "some-prefix/",
},
"key": "folder-4",
"type": "text",
Expand Down Expand Up @@ -388,7 +388,7 @@ exports[`getActionViewTableData should return correct table data for all task st
},
{
"content": {
"text": "/",
"text": "some-prefix/",
},
"key": "folder-5",
"type": "text",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { getPercentValue } from './getPercentValue';
import { getDefaultActionViewHeaders } from './getDefaultActionViewHeaders';
import { ActionViewHeaders } from './types';
import { DefaultActionViewDisplayText } from '../../displayText/types';
import { getFilePath } from '../../actions/handlers';

const getTaskStatusDisplayLabel = ({
status,
Expand Down Expand Up @@ -110,8 +111,12 @@ export const getActionViewTableData = <T extends TaskData = TaskData>({
};
}
case 'folder': {
if (locationKey) {
return { key, type: 'text', content: { text: locationKey } };
if (isFileDataItem(data)) {
return {
key,
type: 'text',
content: { text: getFilePath(data.key) },
};
}

if (isFileItem(data)) {
Expand All @@ -130,6 +135,10 @@ export const getActionViewTableData = <T extends TaskData = TaskData>({
};
}

if (locationKey) {
return { key, type: 'text', content: { text: locationKey } };
}

return { key, type: 'text', content: { text: '/' } };
}
case 'type': {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { LocationPermissions } from '../../../../actions';
import { getFileRowContent } from '../getFileRowContent';
import { LOCATION_DETAIL_VIEW_HEADERS } from '../constants';

describe('getFileRowContent', () => {
const location = {
Expand All @@ -20,13 +21,12 @@ describe('getFileRowContent', () => {
size: 1,
type: 'FILE',
} as const;
const itemLocationKey = `${location.current.prefix}${location.path}`;
Copy link
Member Author

@pranavosu pranavosu Nov 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure if this could break something


it('should return file row content as expected', () => {
expect(
getFileRowContent({
headers: LOCATION_DETAIL_VIEW_HEADERS,
permissions: location.current.permissions,
itemLocationKey,
isSelected: false,
getDateDisplayValue: (date) => date.toLocaleString(),
lastModified: fileItem.lastModified,
Expand Down Expand Up @@ -76,7 +76,7 @@ describe('getFileRowContent', () => {
it('should not render download button if location permission does not support download', () => {
const row = getFileRowContent({
permissions: ['list', 'write'],
itemLocationKey,
headers: LOCATION_DETAIL_VIEW_HEADERS,
isSelected: false,
lastModified: fileItem.lastModified,
rowId: 'row-id',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { getFolderRowContent } from '../getFolderRowContent';
import { LOCATION_DETAIL_VIEW_HEADERS } from '../constants';

describe('getFolderRowContent', () => {
const folderItem = {
Expand All @@ -10,7 +11,8 @@ describe('getFolderRowContent', () => {
it('should return folder row content as expected', () => {
expect(
getFolderRowContent({
itemSubPath: folderItem.key,
headers: LOCATION_DETAIL_VIEW_HEADERS,
rowKey: folderItem.key,
rowId: 'row-id',
onNavigate: jest.fn(),
})
Expand All @@ -19,7 +21,7 @@ describe('getFolderRowContent', () => {
expect.objectContaining({ type: 'text', content: { text: '' } }),
expect.objectContaining({
type: 'button',
content: expect.objectContaining({ label: folderItem.key }),
content: expect.objectContaining({ label: 'path' }),
}),
expect.objectContaining({ type: 'text', content: { text: 'Folder' } }),
expect.objectContaining({ type: 'text', content: { text: '' } }),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ describe('getLocationDetailViewTableData', () => {
areAllFilesSelected: false,
location,
hasFiles: true,
hasSearchResults: false,
pageItems: [folderItem, folderItem, fileItem, fileItem, fileItem],
onDownload: mockOnDownload,
getDateDisplayValue: (date: Date) => date.toLocaleString(),
Expand Down Expand Up @@ -128,6 +129,7 @@ describe('getLocationDetailViewTableData', () => {
areAllFilesSelected: false,
location,
hasFiles: true,
hasSearchResults: false,
pageItems: [folderItem, fileItem],
selectAllFilesLabel: 'Select all files',
selectFileLabel: 'Select file',
Expand All @@ -150,6 +152,7 @@ describe('getLocationDetailViewTableData', () => {
areAllFilesSelected: false,
location,
hasFiles: true,
hasSearchResults: false,
pageItems: [fileItem],
selectAllFilesLabel: 'Select all files',
selectFileLabel: 'Select file',
Expand All @@ -172,6 +175,7 @@ describe('getLocationDetailViewTableData', () => {
areAllFilesSelected: false,
location,
hasFiles: true,
hasSearchResults: false,
pageItems: [fileItem],
selectAllFilesLabel: 'Select all files',
selectFileLabel: 'Select file',
Expand All @@ -194,6 +198,7 @@ describe('getLocationDetailViewTableData', () => {
areAllFilesSelected: false,
location,
hasFiles: true,
hasSearchResults: false,
pageItems: [folderItem],
selectAllFilesLabel: 'Select all files',
selectFileLabel: 'Select file',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,13 @@ import { DataTableProps } from '../../../composables/DataTable';
import { LOCATION_DETAIL_VIEW_HEADERS } from './constants';
import { LocationPermissions } from '../../../actions';
import { LocationDetailViewHeaders } from './types';
import { getFileKey, getFilePath } from '../../../actions/handlers';

export const getFileRowContent = ({
permissions,
isSelected,
itemLocationKey,
getDateDisplayValue,
lastModified,
path,
name,
rowId,
rowKey,
selectFileLabel,
Expand All @@ -24,14 +22,10 @@ export const getFileRowContent = ({
}: {
permissions: LocationPermissions;
isSelected: boolean;
itemLocationKey: string;
lastModified: Date;
headers: LocationDetailViewHeaders;
hasSearchResults: boolean;
getDateDisplayValue: (date: Date) => string;
rowId: string;
path: string;
name?: string;
rowKey: string;
selectFileLabel: string;
size: number;
Expand Down Expand Up @@ -60,7 +54,7 @@ export const getFileRowContent = ({
content: {
icon: 'file',
ariaLabel: 'file',
text: name ?? rowKey.slice(itemLocationKey.length),
text: getFileKey(rowKey),
},
};
}
Expand All @@ -69,7 +63,7 @@ export const getFileRowContent = ({
key,
type: 'text',
content: {
text: path,
text: getFilePath(rowKey),
},
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { DataTableProps } from '../../../composables/DataTable';
import { LOCATION_DETAIL_VIEW_HEADERS } from './constants';
import { LocationDetailViewHeaders } from './types';
import { getFolderName, getFolderPath } from '../../../actions/handlers';

export const getFolderRowContent = ({
itemSubPath,
rowId,
path,
name,
rowKey,
headers,
onNavigate,
}: {
itemSubPath: string;
rowId: string;
path: string;
name?: string;
rowKey: string;
headers: LocationDetailViewHeaders;
onNavigate: () => void;
}): DataTableProps['rows'][number]['content'] =>
Expand All @@ -29,8 +25,8 @@ export const getFolderRowContent = ({
type: 'button',
content: {
icon: 'folder',
ariaLabel: name ?? itemSubPath,
label: name ?? itemSubPath,
ariaLabel: getFolderName(rowKey),
label: getFolderName(rowKey),
onClick: onNavigate,
},
};
Expand All @@ -40,7 +36,7 @@ export const getFolderRowContent = ({
key,
type: 'text',
content: {
text: path,
text: getFolderPath(rowKey),
},
};
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,26 +65,19 @@ export const getLocationDetailViewTableData = ({

const [noopCheckbox, nameHeader, ...rest] = LOCATION_DETAIL_VIEW_HEADERS;

const headers: LocationDetailViewHeaders = [];

if (hasFiles) {
headers.push(headerCheckbox, nameHeader);
} else {
headers.push(noopCheckbox, nameHeader);
}

if (hasSearchResults) {
headers.push(pathHeader);
}

headers.push(...rest);
const headers: LocationDetailViewHeaders = [
hasFiles ? headerCheckbox : noopCheckbox,
nameHeader,
...(hasSearchResults ? [pathHeader] : []),
...rest,
];

const rows: DataTableProps['rows'] = pageItems.map((locationItem) => {
const { id, key, type, name } = locationItem;
const { id, key, type } = locationItem;
switch (type) {
case 'FILE': {
const { lastModified, size } = locationItem;
const { current, path } = location;
const { current } = location;
const isSelected =
fileDataItems?.some((item) => item.id === id) ?? false;
const onFileDownload = () => {
Expand All @@ -96,13 +89,9 @@ export const getLocationDetailViewTableData = ({
return {
key: id,
content: getFileRowContent({
hasSearchResults,
headers,
permissions: current?.permissions ?? [],
isSelected,
path: locationItem.path ?? '',
name,
itemLocationKey: `${current?.prefix ?? ''}${path}`,
lastModified,
getDateDisplayValue,
rowId: id,
Expand All @@ -115,8 +104,7 @@ export const getLocationDetailViewTableData = ({
};
}
case 'FOLDER': {
const { current, path } = location;
const itemSubPath = key.slice(`${current?.prefix ?? ''}${path}`.length);
const { current } = location;
const itemLocationPath = key.slice(current?.prefix.length);
const onFolderNavigate = () => {
if (!current) {
Expand All @@ -127,10 +115,8 @@ export const getLocationDetailViewTableData = ({
return {
key: id,
content: getFolderRowContent({
path: locationItem.path ?? '',
headers,
itemSubPath,
name,
rowKey: key,
rowId: id,
onNavigate: onFolderNavigate,
}),
Expand Down