From a18b2ae88d53eafb8b9d9a7d40fb66c57bbdbc72 Mon Sep 17 00:00:00 2001 From: Devon Govett Date: Thu, 8 Apr 2021 11:45:30 -0600 Subject: [PATCH 1/4] Improve initial drop target in collections when keyboard dragging (#1735) * Improve initial drop target in collections when keyboard dragging * Add tests * Fix React 17 * Fix test wording Co-authored-by: Danni --- packages/@react-aria/dnd/src/DragManager.ts | 4 +- .../dnd/src/useDroppableCollection.ts | 65 ++++++- .../@react-aria/dnd/stories/DroppableGrid.tsx | 4 +- .../dnd/stories/DroppableListBox.tsx | 1 + .../@react-aria/dnd/stories/Reorderable.tsx | 1 + .../dnd/stories/VirtualizedListBox.tsx | 1 + .../dnd/test/useDroppableCollection.test.js | 184 ++++++++++++++++++ .../dnd/src/useDroppableCollectionState.ts | 6 +- 8 files changed, 259 insertions(+), 7 deletions(-) diff --git a/packages/@react-aria/dnd/src/DragManager.ts b/packages/@react-aria/dnd/src/DragManager.ts index f4d4d0df19e..c16b3a0c679 100644 --- a/packages/@react-aria/dnd/src/DragManager.ts +++ b/packages/@react-aria/dnd/src/DragManager.ts @@ -119,7 +119,9 @@ const CANCELED_EVENTS = [ 'touchstart', 'touchmove', 'touchend', - 'keyup' + 'keyup', + 'focusin', + 'focusout' ]; const CLICK_EVENTS = [ diff --git a/packages/@react-aria/dnd/src/useDroppableCollection.ts b/packages/@react-aria/dnd/src/useDroppableCollection.ts index 0d5b2946fc6..5dd6ded7bd4 100644 --- a/packages/@react-aria/dnd/src/useDroppableCollection.ts +++ b/packages/@react-aria/dnd/src/useDroppableCollection.ts @@ -120,6 +120,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let nextKey = target.type === 'item' ? keyboardDelegate.getKeyBelow(target.key) : keyboardDelegate.getFirstKey(); + let dropPosition: DropPosition = 'before'; if (target.type === 'item') { let positionIndex = DROP_POSITIONS.indexOf(target.dropPosition); @@ -131,6 +132,12 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: dropPosition: nextDropPosition }; } + + // If the last drop position was 'after', then 'before' on the next key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === 'after') { + dropPosition = 'on'; + } } if (nextKey == null) { @@ -146,7 +153,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: return { type: 'item', key: nextKey, - dropPosition: DROP_POSITIONS[0] + dropPosition }; }; @@ -155,6 +162,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: let nextKey = target?.type === 'item' ? keyboardDelegate.getKeyAbove(target.key) : keyboardDelegate.getLastKey(); + let dropPosition: DropPosition = !target || target.type === 'root' ? 'after' : 'on'; if (target?.type === 'item') { let positionIndex = DROP_POSITIONS.indexOf(target.dropPosition); @@ -166,6 +174,12 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: dropPosition: nextDropPosition }; } + + // If the last drop position was 'before', then 'after' on the previous key is equivalent. + // Switch to 'on' instead. + if (target.dropPosition === 'before') { + dropPosition = 'on'; + } } if (nextKey == null) { @@ -181,7 +195,7 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: return { type: 'item', key: nextKey, - dropPosition: !target || target.type === 'root' ? 'after' : 'on' + dropPosition }; }; @@ -231,7 +245,52 @@ export function useDroppableCollection(props: DroppableCollectionOptions, state: }, onDropEnter(e, drag) { let types = getTypes(drag.items); - let target = nextValidTarget(null, types, drag.allowedDropOperations, getNextTarget); + let selectionManager = localState.state.selectionManager; + let target: DropTarget; + + // When entering the droppable collection for the first time, the default drop target + // is after the focused key. + let key = selectionManager.focusedKey; + let dropPosition: DropPosition = 'after'; + + // If the focused key is a cell, get the parent item instead. + // For now, we assume that individual cells cannot be dropped on. + let item = localState.state.collection.getItem(key); + if (item?.type === 'cell') { + key = item.parentKey; + } + + // If the focused item is also selected, the default drop target is after the last selected item. + // But if the focused key is the first selected item, then default to before the first selected item. + // This is to make reordering lists slightly easier. If you select top down, we assume you want to + // move the items down. If you select bottom up, we assume you want to move the items up. + if (selectionManager.isSelected(key)) { + if (selectionManager.selectedKeys.size > 1 && selectionManager.firstSelectedKey === key) { + dropPosition = 'before'; + } else { + key = selectionManager.lastSelectedKey; + } + } + + if (key != null) { + target = { + type: 'item', + key, + dropPosition + }; + + // If the default target is not valid, find the next one that is. + if (localState.state.getDropOperation(target, types, drag.allowedDropOperations) === 'cancel') { + target = nextValidTarget(target, types, drag.allowedDropOperations, getNextTarget, false) + ?? nextValidTarget(target, types, drag.allowedDropOperations, getPreviousTarget, false); + } + } + + // If no focused key, then start from the root. + if (!target) { + target = nextValidTarget(null, types, drag.allowedDropOperations, getNextTarget); + } + localState.state.setTarget(target); }, onDropExit() { diff --git a/packages/@react-aria/dnd/stories/DroppableGrid.tsx b/packages/@react-aria/dnd/stories/DroppableGrid.tsx index 1742e2d07fb..988bf974e64 100644 --- a/packages/@react-aria/dnd/stories/DroppableGrid.tsx +++ b/packages/@react-aria/dnd/stories/DroppableGrid.tsx @@ -149,6 +149,7 @@ function DroppableGrid(props) { let dropState = useDroppableCollectionState({ collection: gridState.collection, + selectionManager: gridState.selectionManager, getDropOperation: props.getDropOperation || defaultGetDropOperation, onDropEnter: props.onDropEnter, onDropMove: props.onDropMove, @@ -282,8 +283,7 @@ function CollectionItem({item, state, dropState, onPaste}) { let {gridCellProps} = useGridCell({ node: cellNode, ref: cellRef, - focusMode: 'cell', - shouldSelectOnPressUp: true + focusMode: 'cell' }, state); let dropIndicatorRef = React.useRef(); diff --git a/packages/@react-aria/dnd/stories/DroppableListBox.tsx b/packages/@react-aria/dnd/stories/DroppableListBox.tsx index 80bc6ae9402..183a455c1ff 100644 --- a/packages/@react-aria/dnd/stories/DroppableListBox.tsx +++ b/packages/@react-aria/dnd/stories/DroppableListBox.tsx @@ -93,6 +93,7 @@ export function DroppableListBox(props) { let dropState = useDroppableCollectionState({ collection: state.collection, + selectionManager: state.selectionManager, getDropOperation(target, types, allowedOperations) { if (target.type === 'root') { return 'move'; diff --git a/packages/@react-aria/dnd/stories/Reorderable.tsx b/packages/@react-aria/dnd/stories/Reorderable.tsx index 4adb90f48e7..a7a49005661 100644 --- a/packages/@react-aria/dnd/stories/Reorderable.tsx +++ b/packages/@react-aria/dnd/stories/Reorderable.tsx @@ -150,6 +150,7 @@ function ReorderableGrid(props) { let dropState = useDroppableCollectionState({ collection: gridState.collection, + selectionManager: gridState.selectionManager, getDropOperation(target) { if (target.type === 'root' || target.dropPosition === 'on') { return 'cancel'; diff --git a/packages/@react-aria/dnd/stories/VirtualizedListBox.tsx b/packages/@react-aria/dnd/stories/VirtualizedListBox.tsx index 968235a76c1..765e9ad0f0e 100644 --- a/packages/@react-aria/dnd/stories/VirtualizedListBox.tsx +++ b/packages/@react-aria/dnd/stories/VirtualizedListBox.tsx @@ -102,6 +102,7 @@ export function VirtualizedListBox(props) { let dropState = useDroppableCollectionState({ collection: state.collection, + selectionManager: state.selectionManager, getDropOperation(target, types, allowedOperations) { if (props.accept) { // Don't accept if types includes both items and folders. diff --git a/packages/@react-aria/dnd/test/useDroppableCollection.test.js b/packages/@react-aria/dnd/test/useDroppableCollection.test.js index 39ce1c263d9..2fb35032b06 100644 --- a/packages/@react-aria/dnd/test/useDroppableCollection.test.js +++ b/packages/@react-aria/dnd/test/useDroppableCollection.test.js @@ -770,6 +770,190 @@ describe('useDroppableCollection', () => { }); } }); + + it('should default to dropping after the last focused item if any', () => { + let onDrop = jest.fn(); + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let grid = tree.getByRole('grid'); + let cells = within(grid).getAllByRole('gridcell'); + expect(cells).toHaveLength(3); + + userEvent.tab(); + userEvent.tab(); + expect(document.activeElement).toBe(cells[0]); + + pressKey('ArrowDown'); + expect(document.activeElement).toBe(cells[1]); + + userEvent.tab({shift: true}); + expect(document.activeElement).toBe(draggable); + + pressKey('Enter'); + act(() => jest.runAllTimers()); + + expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between Two and Three'); + }); + + it('should default to dropping after the selected items if any', () => { + let onDrop = jest.fn(); + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let grid = tree.getByRole('grid'); + let cells = within(grid).getAllByRole('gridcell'); + let rows = within(grid).getAllByRole('row'); + expect(cells).toHaveLength(3); + + userEvent.tab(); + userEvent.tab(); + pressKey(' '); + expect(document.activeElement).toBe(cells[0]); + expect(rows[0]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey(' '); + expect(document.activeElement).toBe(cells[2]); + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowUp'); + pressKey(' '); + expect(document.activeElement).toBe(cells[1]); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + + userEvent.tab({shift: true}); + expect(document.activeElement).toBe(draggable); + + pressKey('Enter'); + act(() => jest.runAllTimers()); + + expect(document.activeElement).toHaveAttribute('aria-label', 'Insert after Three'); + }); + + it('should default to before the selected items if the last focused item is the first selected item', () => { + let onDrop = jest.fn(); + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let grid = tree.getByRole('grid'); + let cells = within(grid).getAllByRole('gridcell'); + let rows = within(grid).getAllByRole('row'); + expect(cells).toHaveLength(3); + + userEvent.tab(); + userEvent.tab(); + expect(document.activeElement).toBe(cells[0]); + + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey(' '); + expect(document.activeElement).toBe(cells[2]); + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowUp'); + pressKey(' '); + expect(document.activeElement).toBe(cells[1]); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + + userEvent.tab({shift: true}); + expect(document.activeElement).toBe(draggable); + + pressKey('Enter'); + act(() => jest.runAllTimers()); + + expect(document.activeElement).toHaveAttribute('aria-label', 'Insert between One and Two'); + }); + + it('should default to on the first selected item if the last focused item is the first selected item and only dropping on items is allowed', () => { + let onDrop = jest.fn(); + let getDropOperation = (target) => target.dropPosition !== 'on' ? 'cancel' : 'move'; + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let grid = tree.getByRole('grid'); + let cells = within(grid).getAllByRole('gridcell'); + let rows = within(grid).getAllByRole('row'); + expect(cells).toHaveLength(3); + + userEvent.tab(); + userEvent.tab(); + expect(document.activeElement).toBe(cells[0]); + + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey(' '); + expect(document.activeElement).toBe(cells[2]); + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowUp'); + pressKey(' '); + expect(document.activeElement).toBe(cells[1]); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + + userEvent.tab({shift: true}); + expect(document.activeElement).toBe(draggable); + + pressKey('Enter'); + act(() => jest.runAllTimers()); + + expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Two'); + }); + + it('should default to on the last selected item when only dropping on items is allowed', () => { + let onDrop = jest.fn(); + let getDropOperation = (target) => target.dropPosition !== 'on' ? 'cancel' : 'move'; + let tree = render(<> + + + ); + + let draggable = tree.getByText('Drag me'); + let grid = tree.getByRole('grid'); + let cells = within(grid).getAllByRole('gridcell'); + let rows = within(grid).getAllByRole('row'); + expect(cells).toHaveLength(3); + + userEvent.tab(); + userEvent.tab(); + pressKey(' '); + expect(document.activeElement).toBe(cells[0]); + expect(rows[0]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowDown'); + pressKey('ArrowDown'); + pressKey(' '); + expect(document.activeElement).toBe(cells[2]); + expect(rows[2]).toHaveAttribute('aria-selected', 'true'); + + pressKey('ArrowUp'); + pressKey(' '); + expect(document.activeElement).toBe(cells[1]); + expect(rows[1]).toHaveAttribute('aria-selected', 'true'); + + userEvent.tab({shift: true}); + expect(document.activeElement).toBe(draggable); + + pressKey('Enter'); + act(() => jest.runAllTimers()); + + expect(document.activeElement).toHaveAttribute('aria-label', 'Drop on Three'); + }); }); describe('screen reader', () => { diff --git a/packages/@react-stately/dnd/src/useDroppableCollectionState.ts b/packages/@react-stately/dnd/src/useDroppableCollectionState.ts index e4c894ce9c6..6bfb7274dc9 100644 --- a/packages/@react-stately/dnd/src/useDroppableCollectionState.ts +++ b/packages/@react-stately/dnd/src/useDroppableCollectionState.ts @@ -11,14 +11,17 @@ */ import {Collection, DragTypes, DropOperation, DroppableCollectionProps, DropTarget, ItemDropTarget, Node} from '@react-types/shared'; +import {MultipleSelectionManager} from '@react-stately/selection'; import {useState} from 'react'; interface DroppableCollectionStateOptions extends DroppableCollectionProps { - collection: Collection> + collection: Collection>, + selectionManager: MultipleSelectionManager } export interface DroppableCollectionState { collection: Collection>, + selectionManager: MultipleSelectionManager, target: DropTarget, setTarget(target: DropTarget): void, isDropTarget(target: DropTarget): boolean, @@ -40,6 +43,7 @@ export function useDroppableCollectionState(props: DroppableCollectionStateOptio return { collection: props.collection, + selectionManager: props.selectionManager, target, setTarget(newTarget) { if (this.isDropTarget(newTarget)) { From 8cd12144e8bc3381f376c4315fbfb513c9fcf2e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20B=C3=BCnger?= Date: Fri, 9 Apr 2021 00:37:02 +0200 Subject: [PATCH 2/4] Fix: useTreeState rebuilds the tree collection when a node is expanded. (#1711) * fix: collection in tree state hook would not get rebuild when toggling an item * adding useTreeState story for testing collection rebuild * Move story to CSF for reuse in testing Co-authored-by: Daniel Co-authored-by: Robert Snow --- .../@react-stately/tree/src/useTreeState.ts | 2 +- .../tree/stories/useTreeState.stories.tsx | 214 ++++++++++++++++++ .../tree/test/useTreeState.test.js | 53 +++++ 3 files changed, 268 insertions(+), 1 deletion(-) create mode 100644 packages/@react-stately/tree/stories/useTreeState.stories.tsx create mode 100644 packages/@react-stately/tree/test/useTreeState.test.js diff --git a/packages/@react-stately/tree/src/useTreeState.ts b/packages/@react-stately/tree/src/useTreeState.ts index 1b1f863c392..55087f9c7f6 100644 --- a/packages/@react-stately/tree/src/useTreeState.ts +++ b/packages/@react-stately/tree/src/useTreeState.ts @@ -51,7 +51,7 @@ export function useTreeState(props: TreeProps): TreeState() , [props.disabledKeys]); - let tree = useCollection(props, nodes => new TreeCollection(nodes, {expandedKeys})); + let tree = useCollection(props, nodes => new TreeCollection(nodes, {expandedKeys}), null, [expandedKeys]); // Reset focused key if that item is deleted from the collection. useEffect(() => { diff --git a/packages/@react-stately/tree/stories/useTreeState.stories.tsx b/packages/@react-stately/tree/stories/useTreeState.stories.tsx new file mode 100644 index 00000000000..e84064e1983 --- /dev/null +++ b/packages/@react-stately/tree/stories/useTreeState.stories.tsx @@ -0,0 +1,214 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {Item} from '@react-stately/collections'; +import {Node} from '@react-types/shared'; +import React, {Key, useMemo, useRef} from 'react'; +import {TreeCollection} from '../src/TreeCollection'; +import {usePress} from '@react-aria/interactions'; +import {useSelectableCollection, useSelectableItem} from '@react-aria/selection'; +import {useTreeState} from '../'; + +export default { + title: 'useTreeState' +}; + +export const KeyboardNavigation = () => ; + + +function TreeExample(props = {}) { + return ( + + + Aardvark + + Black Bear + Brown Bear + + Kangaroo + Snake + + + Apple + Orange + + Golden Kiwi + Fuzzy Kiwi + + + + ); +} + +function Tree(props) { + let state = useTreeState(props); + let ref = useRef(); + + let keyboardDelegate = useMemo(() => new TreeKeyboardDelegate(state.collection, state.disabledKeys), [state.collection, state.disabledKeys]); + + let {collectionProps} = useSelectableCollection({ + keyboardDelegate, + ref, + selectionManager: state.selectionManager + }); + + return ( +
+ {TreeNodes({nodes: state.collection, state})} +
+ ); +} + +function TreeNodes({nodes, state}) { + return Array.from(nodes).map((node: Node) => ( + + )); +} + +function TreeItem({node, state}) { + let ref = useRef(); + + let {itemProps} = useSelectableItem({ + key: node.key, + selectionManager: state.selectionManager, + ref: ref + }); + + let {pressProps} = usePress({ + ...itemProps, + onPress: () => state.toggleKey(node.key) + }); + + let isExpanded = node.hasChildNodes && state.expandedKeys.has(node.key); + let isSelected = state.selectionManager.isSelected(node.key); + + return ( +
+
+ {node.rendered} +
+ {isExpanded && +
+ {TreeNodes({nodes: node.childNodes, state})} +
+ } +
+ ); +} + +class TreeKeyboardDelegate { + collator: Intl.Collator; + collection: TreeCollection; + disabledKeys: Set; + + constructor(collection, disabledKeys) { + this.collator = new Intl.Collator('en'); + this.collection = collection; + this.disabledKeys = disabledKeys; + } + + getKeyAbove(key) { + let {collection, disabledKeys} = this; + let keyBefore = collection.getKeyBefore(key); + + while (keyBefore !== null) { + let item = collection.getItem(keyBefore); + + if (item?.type === 'item' && !disabledKeys.has(item.key)) { + return keyBefore; + } + + keyBefore = collection.getKeyBefore(keyBefore); + } + + return null; + } + + getKeyBelow(key) { + let {collection, disabledKeys} = this; + let keyBelow = collection.getKeyAfter(key); + + while (keyBelow !== null) { + let item = collection.getItem(keyBelow); + + if (item?.type === 'item' && !disabledKeys.has(item.key)) { + return keyBelow; + } + + keyBelow = collection.getKeyAfter(keyBelow); + } + + return null; + } + + getFirstKey() { + let {collection, disabledKeys} = this; + let key = collection.getFirstKey(); + + while (key !== null) { + let item = collection.getItem(key); + + if (item?.type === 'item' && !disabledKeys.has(item.key)) { + return key; + } + + key = collection.getKeyAfter(key); + } + + return null; + } + + getLastKey() { + let {collection, disabledKeys} = this; + let key = collection.getLastKey(); + + while (key !== null) { + let item = collection.getItem(key); + + if (item?.type === 'item' && !disabledKeys.has(item.key)) { + return key; + } + + key = collection.getKeyBefore(key); + } + + return null; + } + + getKeyForSearch(search, fromKey = this.getFirstKey()) { + let {collator, collection} = this; + let key = fromKey; + + while (key !== null) { + let item = collection.getItem(key); + + if (item?.textValue && collator.compare(search, item.textValue.slice(0, search.length)) === 0) { + return key; + } + + key = this.getKeyBelow(key); + } + + return null; + } +} diff --git a/packages/@react-stately/tree/test/useTreeState.test.js b/packages/@react-stately/tree/test/useTreeState.test.js new file mode 100644 index 00000000000..f750e0a4d65 --- /dev/null +++ b/packages/@react-stately/tree/test/useTreeState.test.js @@ -0,0 +1,53 @@ +/* + * Copyright 2020 Adobe. All rights reserved. + * This file is licensed to you under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. You may obtain a copy + * of the License at http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software distributed under + * the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS + * OF ANY KIND, either express or implied. See the License for the specific language + * governing permissions and limitations under the License. + */ + +import {fireEvent, render} from '@testing-library/react'; +import {KeyboardNavigation} from '../stories/useTreeState.stories'; +import React from 'react'; +import userEvent from '@testing-library/user-event'; + +describe('useTreeState', () => { + it('should be keyboard navigable', () => { + let {getAllByRole} = render(); + userEvent.tab(); + let items = getAllByRole('treeitem'); + expect(items.length).toBe(2); + expect(document.activeElement).toBe(items[0]); + + // at Animals, expand + // don't bother to check the text, what matters is where we are positionally + fireEvent.keyDown(document.activeElement, {key: 'Enter'}); + fireEvent.keyUp(document.activeElement, {key: 'Enter'}); + items = getAllByRole('treeitem'); + expect(items.length).toBe(6); + expect(document.activeElement).toBe(items[0]); + + // move to Aardvark + fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); + expect(document.activeElement).toBe(items[1]); + + fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); + // at Bear now, expand again + fireEvent.keyDown(document.activeElement, {key: 'Enter'}); + fireEvent.keyUp(document.activeElement, {key: 'Enter'}); + items = getAllByRole('treeitem'); + expect(items.length).toBe(8); + expect(document.activeElement).toBe(items[2]); + + // move to Black Bear + fireEvent.keyDown(document.activeElement, {key: 'ArrowDown'}); + fireEvent.keyUp(document.activeElement, {key: 'ArrowDown'}); + expect(document.activeElement).toBe(items[3]); + }); +}); From 8f95207c965107453588745725670b1e33a3c54b Mon Sep 17 00:00:00 2001 From: Taiki Ikeda <46433895+so99ynoodles@users.noreply.github.com> Date: Fri, 9 Apr 2021 08:10:38 +0900 Subject: [PATCH 3/4] spectrum-Search-icon should be pushed down into TextFieldBase (#1685) * Fix to use spectrum-Textfield-icon instaed of spectrum-Search-icon * Delete UNSAFE_className * small adjustment to left position of the icon which drifted too far to the start edge * Update to match padding from spectrum-css We need to check in with design on this before we can merge it * fix start padding to rely on textfield Co-authored-by: Rob Snow --- .../components/search/index.css | 15 ------- .../components/search/skin.css | 42 ------------------- .../components/search/vars.css | 1 - .../components/textfield/index.css | 10 ++++- .../searchfield/src/SearchField.tsx | 9 +--- 5 files changed, 9 insertions(+), 68 deletions(-) delete mode 100644 packages/@adobe/spectrum-css-temp/components/search/skin.css diff --git a/packages/@adobe/spectrum-css-temp/components/search/index.css b/packages/@adobe/spectrum-css-temp/components/search/index.css index c403fffc7ad..ba56ce3d9c7 100644 --- a/packages/@adobe/spectrum-css-temp/components/search/index.css +++ b/packages/@adobe/spectrum-css-temp/components/search/index.css @@ -70,8 +70,6 @@ governing permissions and limitations under the License. /* Correct the outline for input[type="search"] style in Safari. */ outline-offset: -2px; - /* Use padding instead of text-indent so long strings don't overlap the icon */ - padding-inline-start: var(--spectrum-search-padding-left); text-indent: 0; /* Don't let long strings overlap the close icon */ @@ -96,16 +94,3 @@ governing permissions and limitations under the License. } } -/* Positions search magnifier UI icon */ -.spectrum-Search .spectrum-Search-icon { - display: block; - position: absolute; - height: var(--spectrum-global-dimension-size-200); - width: var(--spectrum-global-dimension-size-200); - inset-inline-start: 12px; - top: calc(calc(var(--spectrum-textfield-height) / 2) - calc(var(--spectrum-global-dimension-size-200) / 2)); - - transition: fill var(--spectrum-global-animation-duration-100) ease-in-out; - - pointer-events: none; -} diff --git a/packages/@adobe/spectrum-css-temp/components/search/skin.css b/packages/@adobe/spectrum-css-temp/components/search/skin.css deleted file mode 100644 index 545e93dc752..00000000000 --- a/packages/@adobe/spectrum-css-temp/components/search/skin.css +++ /dev/null @@ -1,42 +0,0 @@ -/* -Copyright 2019 Adobe. All rights reserved. -This file is licensed to you under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. You may obtain a copy -of the License at http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software distributed under -the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS -OF ANY KIND, either express or implied. See the License for the specific language -governing permissions and limitations under the License. -*/ - -.spectrum-Search-icon { - fill: var(--spectrum-textfield-icon-color); -} - -.spectrum-Search-input { - /* The value of color is identical for hover/active/focus-ring, but we repeat it here in case one is changed in the future */ - &:hover { - & ~ .spectrum-Search-icon { - fill: var(--spectrum-search-icon-color-hover); - } - } - - &:active { - & ~ .spectrum-Search-icon { - fill: var(--spectrum-search-icon-color-down); - } - } - - &:focus-ring { - & ~ .spectrum-Search-icon { - fill: var(--spectrum-search-icon-color-key-focus); - } - } - - &:disabled { - & ~ .spectrum-Search-icon { - fill: var(--spectrum-textfield-text-color-disabled); - } - } -} diff --git a/packages/@adobe/spectrum-css-temp/components/search/vars.css b/packages/@adobe/spectrum-css-temp/components/search/vars.css index ca1e08522d8..3df0b027c58 100644 --- a/packages/@adobe/spectrum-css-temp/components/search/vars.css +++ b/packages/@adobe/spectrum-css-temp/components/search/vars.css @@ -11,4 +11,3 @@ */ @import './index.css'; -@import './skin.css'; diff --git a/packages/@adobe/spectrum-css-temp/components/textfield/index.css b/packages/@adobe/spectrum-css-temp/components/textfield/index.css index 28ae069f5fd..7f49de02c5d 100644 --- a/packages/@adobe/spectrum-css-temp/components/textfield/index.css +++ b/packages/@adobe/spectrum-css-temp/components/textfield/index.css @@ -227,7 +227,8 @@ governing permissions and limitations under the License. position: absolute; height: var(--spectrum-icon-info-medium-height); width: var(--spectrum-icon-info-medium-width); - inset-inline-start: var(--spectrum-global-dimension-size-100); + /* This has a named variable in a future update of spectrum-css. */ + inset-inline-start: var(--spectrum-global-dimension-size-150); top: var(--spectrum-global-dimension-size-85); transition: fill var(--spectrum-global-animation-duration-100) ease-in-out; } @@ -235,7 +236,12 @@ governing permissions and limitations under the License. /* styles the textfield properly if the left icon is provided */ .spectrum-Textfield-inputIcon { /* Use padding instead of text-indent so long strings don't overlap the icon */ - padding-inline-start: calc(calc(var(--spectrum-global-dimension-size-200) + var(--spectrum-icon-info-medium-width)) - 1px); + /* These values have real names in a spectrum-css update, when we update, use those. */ + padding-inline-start: calc( + var(--spectrum-global-dimension-size-150) + + var(--spectrum-global-dimension-size-225) + + var(--spectrum-global-dimension-size-65) + ); .spectrum-Textfield--quiet & { padding-inline-start: calc(var(--spectrum-global-dimension-size-100) + var(--spectrum-icon-info-medium-width)); diff --git a/packages/@react-spectrum/searchfield/src/SearchField.tsx b/packages/@react-spectrum/searchfield/src/SearchField.tsx index 1b33438bf55..cbd1f5997a1 100644 --- a/packages/@react-spectrum/searchfield/src/SearchField.tsx +++ b/packages/@react-spectrum/searchfield/src/SearchField.tsx @@ -25,14 +25,7 @@ import {useSearchFieldState} from '@react-stately/searchfield'; function SearchField(props: SpectrumSearchFieldProps, ref: RefObject) { props = useProviderProps(props); let defaultIcon = ( - + ); let { From 887fff79e8f33c0f8953dc38955419feddd56b6a Mon Sep 17 00:00:00 2001 From: Robert Snow Date: Thu, 8 Apr 2021 18:01:59 -0700 Subject: [PATCH 4/4] Discourage use of useDrag1D in favor of useMove (#1757) * Add warning to discourage use of useDrag1D in favor of useMove --- packages/@react-aria/utils/src/useDrag1D.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/@react-aria/utils/src/useDrag1D.ts b/packages/@react-aria/utils/src/useDrag1D.ts index 90dc8c727d8..618cdf0c61e 100644 --- a/packages/@react-aria/utils/src/useDrag1D.ts +++ b/packages/@react-aria/utils/src/useDrag1D.ts @@ -39,6 +39,7 @@ const draggingElements: HTMLElement[] = []; // It can also handle either a vertical or horizontal movement, but not both at the same time export function useDrag1D(props: UseDrag1DProps): HTMLAttributes { + console.warn('useDrag1D is deprecated, please use `useMove` instead https://react-spectrum.adobe.com/react-aria/useMove.html'); let {containerRef, reverse, orientation, onHover, onDrag, onPositionChange, onIncrement, onDecrement, onIncrementToMax, onDecrementToMin, onCollapseToggle} = props; let getPosition = (e) => orientation === 'horizontal' ? e.clientX : e.clientY; let getNextOffset = (e: MouseEvent) => {