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

fix: Prevent dragging options in Collection preferences from causing page overflow #1072

Merged
merged 12 commits into from
May 9, 2023
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,16 @@ import useBrowser from '@cloudscape-design/browser-test-tools/use-browser';
import createWrapper from '../../../../lib/components/test-utils/selectors';
import ContentDisplayPageObject from './pages/content-display-page';

const setupTest = (testFn: (page: ContentDisplayPageObject) => Promise<void>, height = 1200) => {
const windowDimensions = {
width: 1200,
height: 1200,
};

const setupTest = (testFn: (page: ContentDisplayPageObject) => Promise<void>) => {
return useBrowser(async browser => {
const page = new ContentDisplayPageObject(browser);
await browser.url('#/light/collection-preferences/reorder-content');
await page.setWindowSize({ width: 1200, height });
await page.setWindowSize(windowDimensions);
await testFn(page);
});
};
Expand Down Expand Up @@ -64,6 +69,43 @@ describe('Collection preferences - Content Display preference', () => {
await expect(wrapper.findModal()).not.toBeNull();
})
);

describe('does not cause overflow when reaching the edge of the window', () => {
const wait = (delay: number) => new Promise(resolve => setTimeout(resolve, delay));
Copy link
Member

Choose a reason for hiding this comment

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

Can we use page.pause() instead?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done


const testByDraggingToPosition = async (page: ContentDisplayPageObject, x: number, y: number) => {
const wrapper = createWrapper().findCollectionPreferences('.cp-1');
await page.openCollectionPreferencesModal(wrapper);
const modal = wrapper.findModal();
const modalContentSelector = wrapper.findModal().findContent().toSelector();
const modalContentBox = await page.getBoundingBox(modalContentSelector);

const dragHandleSelector = modal
.findContentDisplayPreference()
.findOptions()
.get(1)
.findDragHandle()
.toSelector();
const dragHandleBox = await page.getBoundingBox(dragHandleSelector);
const delta = { x: x - dragHandleBox.right, y: y - dragHandleBox.bottom };
await page.mouseDown(dragHandleSelector);
await page.mouseMove(Math.round(delta.x), Math.round(delta.y));
await wait(100);

const newModalContentBox = await page.getBoundingBox(modalContentSelector);
expect(newModalContentBox).toEqual(modalContentBox);
};

test(
'horizontally',
setupTest(page => testByDraggingToPosition(page, windowDimensions.width, windowDimensions.height / 2))
);

test(
'vertically',
setupTest(page => testByDraggingToPosition(page, windowDimensions.width / 2, windowDimensions.height))
);
});
});

describe('reorders content with keyboard', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,60 +21,54 @@ $border-radius: awsui.$border-radius-item;
}
}

.sortable-item-toggle {
.content-display-option-toggle {
Copy link
Member Author

Choose a reason for hiding this comment

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

Harmonized internal names with testing utils API: "sortable item" is now "content display option".

/* used in test-utils */
}

.sortable-item {
position: relative;
}

.sortable-item-placeholder {
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
background: awsui.$color-drag-placeholder-hover;
.content-display-option-content {
@include styles.styles-reset;
display: flex;
align-items: flex-start;
padding: awsui.$space-xs awsui.$space-scaled-xs awsui.$space-xs 0;
background-color: awsui.$color-background-container-content;
border-radius: $border-radius;
}

.sortable-item-content {
.content-display-option {
list-style: none;
position: relative;
border-top: awsui.$border-divider-list-width solid awsui.$color-border-divider-default;
display: flex;
flex-wrap: nowrap;
justify-content: space-between;
align-items: flex-start;
padding-top: awsui.$space-xs;
padding-bottom: awsui.$space-xs;
padding-right: 0;
&:not(.draggable) {
padding-left: awsui.$space-scaled-l;
}
&.draggable {
padding-left: 0;
padding-right: awsui.$space-scaled-xs;
background-color: awsui.$color-background-container-content;
&:not(.placeholder).sorting {
@include animated;
z-index: 1;
&.active {
}
&.placeholder {
> .content-display-option-content {
position: relative;
z-index: 2;
box-shadow: awsui.$shadow-container-active;
border-radius: $border-radius;
}
&:not(.active).sorting {
@include animated;
}
@include focus-visible {
&.active {
@include animated;
@include styles.focus-highlight(0px);
&:after {
content: ' ';
position: absolute;
top: 0;
bottom: 0;
left: 0;
right: 0;
background: awsui.$color-drag-placeholder-hover;
border-radius: $border-radius;
}
}
}
}

.sortable-item-label {
padding-right: awsui.$space-l;
.content-display-option-label {
flex-grow: 1;
overflow: hidden;
padding-right: awsui.$space-l;
}

.drag-overlay {
box-shadow: awsui.$shadow-container-active;
border-radius: $border-radius;
@include focus-visible {
@include styles.focus-highlight(0px, $border-radius);
Copy link
Member Author

Choose a reason for hiding this comment

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

The second parameter $border-radius was previously missing; this fixes the focus ring border radius.

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import styles from '../styles.css.js';
import DragHandle from '../../internal/drag-handle';
import InternalToggle from '../../toggle/internal';
import React, { ForwardedRef, forwardRef } from 'react';
import { SyntheticListenerMap } from '@dnd-kit/core/dist/hooks/utilities';
import { OptionWithVisibility } from './utils';
import { useUniqueId } from '../../internal/hooks/use-unique-id';

const componentPrefix = 'content-display-option';
export const getClassName = (suffix?: string) => styles[[componentPrefix, suffix].filter(Boolean).join('-')];

export interface ContentDisplayItemProps {
dragHandleAriaLabel?: string;
listeners?: SyntheticListenerMap;
onToggle?: (option: OptionWithVisibility) => void;
option: OptionWithVisibility;
}

const ContentDisplayOption = forwardRef(
(
{ dragHandleAriaLabel, listeners, onToggle, option }: ContentDisplayItemProps,
ref: ForwardedRef<HTMLDivElement>
) => {
const idPrefix = useUniqueId(componentPrefix);
const controlId = `${idPrefix}-control-${option.id}`;

const dragHandleAttributes = {
['aria-label']: [dragHandleAriaLabel, option.label].join(', '),
};

return (
<div ref={ref} className={getClassName('content')}>
<DragHandle attributes={dragHandleAttributes} listeners={listeners} />

<label className={getClassName('label')} htmlFor={controlId}>
{option.label}
</label>
<div className={getClassName('toggle')}>
<InternalToggle
checked={!!option.visible}
onChange={() => onToggle && onToggle(option)}
disabled={option.alwaysVisible === true}
controlId={controlId}
/>
</div>
</div>
);
}
);

export default ContentDisplayOption;
52 changes: 52 additions & 0 deletions src/collection-preferences/content-display/draggable-option.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
// SPDX-License-Identifier: Apache-2.0
import React from 'react';
import { useSortable } from '@dnd-kit/sortable';
import { CSS } from '@dnd-kit/utilities';
import { OptionWithVisibility } from './utils';
import ContentDisplayOption, { getClassName } from './content-display-option';
import clsx from 'clsx';
import styles from '../styles.css.js';

export default function DraggableOption({
dragHandleAriaLabel,
onKeyDown,
onToggle,
option,
}: {
dragHandleAriaLabel?: string;
onKeyDown?: (event: React.KeyboardEvent) => void;
onToggle: (option: OptionWithVisibility) => void;
option: OptionWithVisibility;
}) {
const { isDragging, isSorting, listeners, setNodeRef, transform } = useSortable({
id: option.id,
});
const style = {
transform: CSS.Translate.toString(transform),
};

const combinedListeners = {
...listeners,
onKeyDown: (event: React.KeyboardEvent) => {
if (onKeyDown) {
onKeyDown(event);
}
if (listeners?.onKeyDown) {
listeners.onKeyDown(event);
}
},
};

return (
<li className={clsx(getClassName(), isDragging && styles.placeholder, isSorting && styles.sorting)} style={style}>
<ContentDisplayOption
ref={setNodeRef}
listeners={combinedListeners}
dragHandleAriaLabel={dragHandleAriaLabel}
onToggle={onToggle}
option={option}
/>
</li>
);
}
44 changes: 34 additions & 10 deletions src/collection-preferences/content-display/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@ import { useUniqueId } from '../../internal/hooks/use-unique-id';
import { CollectionPreferencesProps } from '../interfaces';
import styles from '../styles.css.js';
import { getSortedOptions, OptionWithVisibility } from './utils';
import { DndContext } from '@dnd-kit/core';
import { DndContext, DragOverlay } from '@dnd-kit/core';
import { arrayMove, SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable';
import { SortableItem } from './sortable-item';
import DraggableOption from './draggable-option';
import useDragAndDropReorder from './use-drag-and-drop-reorder';
import useLiveAnnouncements from './use-live-announcements';
import Portal from '../../internal/components/portal';
import ContentDisplayOption from './content-display-option';

const componentPrefix = 'content-display';

Expand Down Expand Up @@ -52,6 +54,8 @@ export default function ContentDisplayPreference({
sortedOptions,
});

const activeOption = activeItem ? sortedOptions.find(({ id }) => id === activeItem) : null;

const announcements = useLiveAnnouncements({
isDragging: activeItem !== null,
liveAnnouncementDndStarted,
Expand Down Expand Up @@ -99,17 +103,37 @@ export default function ContentDisplayPreference({
role="list"
>
<SortableContext items={sortedOptions.map(({ id }) => id)} strategy={verticalListSortingStrategy}>
{sortedOptions.map(option => (
<SortableItem
{sortedOptions.map(option => {
return (
<DraggableOption
dragHandleAriaLabel={dragHandleAriaLabel}
key={option.id}
onKeyDown={handleKeyDown}
onToggle={onToggle}
option={option}
/>
);
})}
</SortableContext>
</ul>
<Portal>
{/* Make sure that the drag overlay is above the modal
by assigning the z-index as inline style
so that it prevails over dnd-kit's inline z-index of 999 */}
{/* className is a documented prop of the DragOverlay component:
https://docs.dndkit.com/api-documentation/draggable/drag-overlay#class-name-and-inline-styles */
/* eslint-disable-next-line react/forbid-component-props */}
<DragOverlay className={styles['drag-overlay']} dropAnimation={null} style={{ zIndex: 5000 }}>
{activeOption && (
<ContentDisplayOption
listeners={{ onKeyDown: handleKeyDown }}
dragHandleAriaLabel={dragHandleAriaLabel}
key={option.id}
onKeyDown={handleKeyDown}
onToggle={onToggle}
option={option}
option={activeOption}
/>
))}
</SortableContext>
</ul>
)}
</DragOverlay>
</Portal>
</DndContext>
</div>
);
Expand Down
Loading