Fix drag-n-drop onto empty space below asset table (#9446)

- The invisible drop target below the assets table was absent, breaking both drag-n-drop of external files, and drag-n-drop of existing assets to the root directory.

# Important Notes
None
This commit is contained in:
somebody1234 2024-03-18 18:47:01 +10:00 committed by GitHub
parent e1893b65af
commit 703cafa6d9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 153 additions and 26 deletions

View File

@ -56,12 +56,12 @@ export function locateUsernameInput(page: test.Locator | test.Page) {
}
/** Find a "name" input for a "new label" modal (if any) on the current page. */
export function locateNewLabelModalNameInput(page: test.Locator | test.Page) {
export function locateNewLabelModalNameInput(page: test.Page) {
return locateNewLabelModal(page).getByLabel('Name')
}
/** Find all color radio button inputs for a "new label" modal (if any) on the current page. */
export function locateNewLabelModalColorButtons(page: test.Locator | test.Page) {
export function locateNewLabelModalColorButtons(page: test.Page) {
return (
locateNewLabelModal(page)
.filter({ has: page.getByText('Color') })
@ -71,17 +71,17 @@ export function locateNewLabelModalColorButtons(page: test.Locator | test.Page)
}
/** Find a "name" input for an "upsert secret" modal (if any) on the current page. */
export function locateSecretNameInput(page: test.Locator | test.Page) {
export function locateSecretNameInput(page: test.Page) {
return locateUpsertSecretModal(page).getByPlaceholder('Enter the name of the secret')
}
/** Find a "value" input for an "upsert secret" modal (if any) on the current page. */
export function locateSecretValueInput(page: test.Locator | test.Page) {
export function locateSecretValueInput(page: test.Page) {
return locateUpsertSecretModal(page).getByPlaceholder('Enter the value of the secret')
}
/** Find a search bar input (if any) on the current page. */
export function locateSearchBarInput(page: test.Locator | test.Page) {
export function locateSearchBarInput(page: test.Page) {
return locateSearchBar(page).getByPlaceholder(
'Type to search for projects, Data Links, users, and more.'
)
@ -157,7 +157,7 @@ export function locateStopProjectButton(page: test.Locator | test.Page) {
}
/** Find all labels in the labels panel (if any) on the current page. */
export function locateLabelsPanelLabels(page: test.Locator | test.Page) {
export function locateLabelsPanelLabels(page: test.Page) {
return (
locateLabelsPanel(page)
.getByRole('button')
@ -192,6 +192,21 @@ export function locateAssetLabels(page: test.Locator | test.Page) {
return page.getByTestId('asset-label')
}
/** Find a toggle for the "Name" column (if any) on the current page. */
export function locateNameColumnToggle(page: test.Locator | test.Page) {
return page.getByAltText(/^(?:Show|Hide) Name$/)
}
/** Find a toggle for the "Modified" column (if any) on the current page. */
export function locateModifiedColumnToggle(page: test.Locator | test.Page) {
return page.getByAltText(/^(?:Show|Hide) Modified date$/)
}
/** Find a toggle for the "Shared with" column (if any) on the current page. */
export function locateSharedWithColumnToggle(page: test.Locator | test.Page) {
return page.getByAltText(/^(?:Show|Hide) Shared with$/)
}
/** Find a toggle for the "Labels" column (if any) on the current page. */
export function locateLabelsColumnToggle(page: test.Locator | test.Page) {
return page.getByAltText(/^(?:Show|Hide) Labels$/)
@ -375,17 +390,17 @@ export function locateAssetPanelIcon(page: test.Locator | test.Page) {
}
/** Find a list of tags in the search bar (if any) on the current page. */
export function locateSearchBarTags(page: test.Locator | test.Page) {
export function locateSearchBarTags(page: test.Page) {
return locateSearchBar(page).getByTestId('asset-search-tag-names').getByRole('button')
}
/** Find a list of labels in the search bar (if any) on the current page. */
export function locateSearchBarLabels(page: test.Locator | test.Page) {
export function locateSearchBarLabels(page: test.Page) {
return locateSearchBar(page).getByTestId('asset-search-labels').getByRole('button')
}
/** Find a list of labels in the search bar (if any) on the current page. */
export function locateSearchBarSuggestions(page: test.Locator | test.Page) {
export function locateSearchBarSuggestions(page: test.Page) {
return locateSearchBar(page).getByTestId('asset-search-suggestion')
}
@ -495,75 +510,89 @@ export function locateCollapsibleDirectories(page: test.Page) {
}
/** Find a "confirm delete" modal (if any) on the current page. */
export function locateConfirmDeleteModal(page: test.Locator | test.Page) {
export function locateConfirmDeleteModal(page: test.Page) {
// This has no identifying features.
return page.getByTestId('confirm-delete-modal')
}
/** Find a "new label" modal (if any) on the current page. */
export function locateNewLabelModal(page: test.Locator | test.Page) {
export function locateNewLabelModal(page: test.Page) {
// This has no identifying features.
return page.getByTestId('new-label-modal')
}
/** Find an "upsert secret" modal (if any) on the current page. */
export function locateUpsertSecretModal(page: test.Locator | test.Page) {
export function locateUpsertSecretModal(page: test.Page) {
// This has no identifying features.
return page.getByTestId('upsert-secret-modal')
}
/** Find a user menu (if any) on the current page. */
export function locateUserMenu(page: test.Locator | test.Page) {
export function locateUserMenu(page: test.Page) {
// This has no identifying features.
return page.getByTestId('user-menu')
}
/** Find a "set username" panel (if any) on the current page. */
export function locateSetUsernamePanel(page: test.Locator | test.Page) {
export function locateSetUsernamePanel(page: test.Page) {
// This has no identifying features.
return page.getByTestId('set-username-panel')
}
/** Find a set of context menus (if any) on the current page. */
export function locateContextMenus(page: test.Locator | test.Page) {
export function locateContextMenus(page: test.Page) {
// This has no identifying features.
return page.getByTestId('context-menus')
}
/** Find a labels panel (if any) on the current page. */
export function locateLabelsPanel(page: test.Locator | test.Page) {
export function locateLabelsPanel(page: test.Page) {
// This has no identifying features.
return page.getByTestId('labels')
}
/** Find a list of labels (if any) on the current page. */
export function locateLabelsList(page: test.Locator | test.Page) {
export function locateLabelsList(page: test.Page) {
// This has no identifying features.
return page.getByTestId('labels-list')
}
/** Find an asset panel (if any) on the current page. */
export function locateAssetPanel(page: test.Locator | test.Page) {
export function locateAssetPanel(page: test.Page) {
// This has no identifying features.
return page.getByTestId('asset-panel')
}
/** Find a search bar (if any) on the current page. */
export function locateSearchBar(page: test.Locator | test.Page) {
export function locateSearchBar(page: test.Page) {
// This has no identifying features.
return page.getByTestId('asset-search-bar')
}
/** Find an extra columns button panel (if any) on the current page. */
export function locateExtraColumns(page: test.Page) {
// This has no identifying features.
return page.getByTestId('extra-columns')
}
/** Find a root directory dropzone (if any) on the current page.
* This is the empty space below the assets table, if it doesn't take up the whole screen
* vertically. */
export function locateRootDirectoryDropzone(page: test.Page) {
// This has no identifying features.
return page.getByTestId('root-directory-dropzone')
}
// === Content locators ===
/** Find an asset description in an asset panel (if any) on the current page. */
export function locateAssetPanelDescription(page: test.Locator | test.Page) {
export function locateAssetPanelDescription(page: test.Page) {
// This has no identifying features.
return locateAssetPanel(page).getByTestId('asset-panel-description')
}
/** Find asset permissions in an asset panel (if any) on the current page. */
export function locateAssetPanelPermissions(page: test.Locator | test.Page) {
export function locateAssetPanelPermissions(page: test.Page) {
// This has no identifying features.
return locateAssetPanel(page).getByTestId('asset-panel-permissions').getByRole('button')
}

View File

@ -0,0 +1,94 @@
/** @file Test the drive view. */
import * as test from '@playwright/test'
import * as actions from './actions'
test.test('extra columns should stick to right side of assets table', async ({ page }) => {
await actions.mockAllAndLogin({ page })
await actions.locateAccessedByProjectsColumnToggle(page).click()
await actions.locateAccessedDataColumnToggle(page).click()
await actions.locateAssetsTable(page).evaluate(element => {
let scrollableParent: HTMLElement | SVGElement | null = element
while (
scrollableParent != null &&
scrollableParent.scrollWidth <= scrollableParent.clientWidth
) {
scrollableParent = scrollableParent.parentElement
}
// eslint-disable-next-line @typescript-eslint/no-magic-numbers
scrollableParent?.scrollTo({ left: 999999, behavior: 'instant' })
})
const extraColumns = actions.locateExtraColumns(page)
const assetsTable = actions.locateAssetsTable(page)
await test
.expect(async () => {
const extraColumnsRight = await extraColumns.evaluate(
element => element.getBoundingClientRect().right
)
const assetsTableRight = await assetsTable.evaluate(
element => element.getBoundingClientRect().right
)
test.expect(extraColumnsRight).toEqual(assetsTableRight)
})
.toPass()
})
test.test('extra columns should stick to top of scroll container', async ({ page }) => {
const { api } = await actions.mockAllAndLogin({ page })
// eslint-disable-next-line @typescript-eslint/no-magic-numbers
for (let i = 0; i < 100; i += 1) {
api.addFile('a')
}
await actions.login({ page })
await actions.locateAccessedByProjectsColumnToggle(page).click()
await actions.locateAccessedDataColumnToggle(page).click()
await actions.locateAssetsTable(page).evaluate(element => {
let scrollableParent: HTMLElement | SVGElement | null = element
while (
scrollableParent != null &&
scrollableParent.scrollWidth <= scrollableParent.clientWidth
) {
scrollableParent = scrollableParent.parentElement
}
// eslint-disable-next-line @typescript-eslint/no-magic-numbers
scrollableParent?.scrollTo({ top: 999999, behavior: 'instant' })
})
const extraColumns = actions.locateExtraColumns(page)
const assetsTable = actions.locateAssetsTable(page)
await test
.expect(async () => {
const extraColumnsTop = await extraColumns.evaluate(
element => element.getBoundingClientRect().top
)
const assetsTableTop = await assetsTable.evaluate(element => {
let scrollableParent: HTMLElement | SVGElement | null = element
while (
scrollableParent != null &&
scrollableParent.scrollWidth <= scrollableParent.clientWidth
) {
scrollableParent = scrollableParent.parentElement
}
return scrollableParent?.getBoundingClientRect().top
})
test.expect(extraColumnsTop).toEqual(assetsTableTop)
})
.toPass()
})
test.test('can drop onto root directory dropzone', async ({ page }) => {
const { api } = await actions.mockAllAndLogin({ page })
const assetRows = actions.locateAssetRows(page)
const asset = api.addDirectory('a')
api.addFile('b', { parentId: asset.id })
await actions.login({ page })
await assetRows.nth(0).dblclick()
const parentLeft = await actions.getAssetRowLeftPx(assetRows.nth(0))
const childLeft = await actions.getAssetRowLeftPx(assetRows.nth(1))
test.expect(childLeft, 'child is indented further than parent').toBeGreaterThan(parentLeft)
await assetRows.nth(1).dragTo(actions.locateRootDirectoryDropzone(page))
const firstLeft = await actions.getAssetRowLeftPx(assetRows.nth(0))
const secondLeft = await actions.getAssetRowLeftPx(assetRows.nth(1))
test.expect(firstLeft, 'siblings have same indentation').toEqual(secondLeft)
})

View File

@ -2186,7 +2186,7 @@ export default function AssetsTable(props: AssetsTableProps) {
const columns = columnUtils.getColumnList(backend.type, enabledColumns)
const headerRow = (
<tr ref={headerRowRef} className="sticky top text-sm font-semibold">
<tr ref={headerRowRef} className="sticky top-[1px] text-sm font-semibold">
{columns.map(column => {
// This is a React component, even though it does not contain JSX.
// eslint-disable-next-line no-restricted-syntax
@ -2422,6 +2422,7 @@ export default function AssetsTable(props: AssetsTableProps) {
</tbody>
</table>
<div
data-testid="root-directory-dropzone"
className="grow"
onClick={() => {
setSelectedKeys(new Set())
@ -2466,10 +2467,13 @@ export default function AssetsTable(props: AssetsTableProps) {
onDragCancel={onSelectionDragCancel}
/>
)}
<div className="w-max">
<div className="flex h-max min-h-full w-max min-w-full flex-col">
{isCloud && (
<div className="sticky top flex h flex-col">
<div className="sticky right flex self-end px-extra-columns-panel-x py-extra-columns-panel-y">
<div className="flex-0 sticky top flex h flex-col">
<div
data-testid="extra-columns"
className="sticky right flex self-end px-extra-columns-panel-x py-extra-columns-panel-y"
>
<div className="inline-flex gap-icons">
{columnUtils.CLOUD_COLUMNS.filter(column => !enabledColumns.has(column)).map(
column => (
@ -2497,7 +2501,7 @@ export default function AssetsTable(props: AssetsTableProps) {
</div>
</div>
)}
<div className="flex h-full w-min min-w-full flex-col">{table}</div>
<div className="flex h-full w-min min-w-full grow flex-col">{table}</div>
</div>
</div>
)