mirror of https://github.com/vector-im/riot-web
Fix roving tab index getting confused after dragging space order (#10901)
* Fix roving tab index getting confused after dragging space order * Fix roving tab index for drag reordering * delint * Add test * Make types happier * Remove snapshotpull/28788/head^2
parent
2da199c41d
commit
d9d53870e3
|
@ -78,16 +78,40 @@ export enum Type {
|
|||
Register = "REGISTER",
|
||||
Unregister = "UNREGISTER",
|
||||
SetFocus = "SET_FOCUS",
|
||||
Update = "UPDATE",
|
||||
}
|
||||
|
||||
export interface IAction {
|
||||
type: Type;
|
||||
type: Exclude<Type, Type.Update>;
|
||||
payload: {
|
||||
ref: Ref;
|
||||
};
|
||||
}
|
||||
|
||||
export const reducer: Reducer<IState, IAction> = (state: IState, action: IAction) => {
|
||||
interface UpdateAction {
|
||||
type: Type.Update;
|
||||
payload?: undefined;
|
||||
}
|
||||
|
||||
type Action = IAction | UpdateAction;
|
||||
|
||||
const refSorter = (a: Ref, b: Ref): number => {
|
||||
if (a === b) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
const position = a.current!.compareDocumentPosition(b.current!);
|
||||
|
||||
if (position & Node.DOCUMENT_POSITION_FOLLOWING || position & Node.DOCUMENT_POSITION_CONTAINED_BY) {
|
||||
return -1;
|
||||
} else if (position & Node.DOCUMENT_POSITION_PRECEDING || position & Node.DOCUMENT_POSITION_CONTAINS) {
|
||||
return 1;
|
||||
} else {
|
||||
return 0;
|
||||
}
|
||||
};
|
||||
|
||||
export const reducer: Reducer<IState, Action> = (state: IState, action: Action) => {
|
||||
switch (action.type) {
|
||||
case Type.Register: {
|
||||
if (!state.activeRef) {
|
||||
|
@ -97,21 +121,7 @@ export const reducer: Reducer<IState, IAction> = (state: IState, action: IAction
|
|||
|
||||
// Sadly due to the potential of DOM elements swapping order we can't do anything fancy like a binary insert
|
||||
state.refs.push(action.payload.ref);
|
||||
state.refs.sort((a, b) => {
|
||||
if (a === b) {
|
||||
return 0;
|
||||
}
|
||||
|
||||
const position = a.current!.compareDocumentPosition(b.current!);
|
||||
|
||||
if (position & Node.DOCUMENT_POSITION_FOLLOWING || position & Node.DOCUMENT_POSITION_CONTAINED_BY) {
|
||||
return -1;
|
||||
} else if (position & Node.DOCUMENT_POSITION_PRECEDING || position & Node.DOCUMENT_POSITION_CONTAINS) {
|
||||
return 1;
|
||||
} else {
|
||||
return 0;
|
||||
}
|
||||
});
|
||||
state.refs.sort(refSorter);
|
||||
|
||||
return { ...state };
|
||||
}
|
||||
|
@ -150,6 +160,11 @@ export const reducer: Reducer<IState, IAction> = (state: IState, action: IAction
|
|||
return { ...state };
|
||||
}
|
||||
|
||||
case Type.Update: {
|
||||
state.refs.sort(refSorter);
|
||||
return { ...state };
|
||||
}
|
||||
|
||||
default:
|
||||
return state;
|
||||
}
|
||||
|
@ -160,7 +175,7 @@ interface IProps {
|
|||
handleHomeEnd?: boolean;
|
||||
handleUpDown?: boolean;
|
||||
handleLeftRight?: boolean;
|
||||
children(renderProps: { onKeyDownHandler(ev: React.KeyboardEvent): void }): ReactNode;
|
||||
children(renderProps: { onKeyDownHandler(ev: React.KeyboardEvent): void; onDragEndHandler(): void }): ReactNode;
|
||||
onKeyDown?(ev: React.KeyboardEvent, state: IState, dispatch: Dispatch<IAction>): void;
|
||||
}
|
||||
|
||||
|
@ -199,7 +214,7 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
|
|||
handleLoop,
|
||||
onKeyDown,
|
||||
}) => {
|
||||
const [state, dispatch] = useReducer<Reducer<IState, IAction>>(reducer, {
|
||||
const [state, dispatch] = useReducer<Reducer<IState, Action>>(reducer, {
|
||||
refs: [],
|
||||
});
|
||||
|
||||
|
@ -301,9 +316,15 @@ export const RovingTabIndexProvider: React.FC<IProps> = ({
|
|||
[context, onKeyDown, handleHomeEnd, handleUpDown, handleLeftRight, handleLoop],
|
||||
);
|
||||
|
||||
const onDragEndHandler = useCallback(() => {
|
||||
dispatch({
|
||||
type: Type.Update,
|
||||
});
|
||||
}, []);
|
||||
|
||||
return (
|
||||
<RovingTabIndexContext.Provider value={context}>
|
||||
{children({ onKeyDownHandler })}
|
||||
{children({ onKeyDownHandler, onDragEndHandler })}
|
||||
</RovingTabIndexContext.Provider>
|
||||
);
|
||||
};
|
||||
|
|
|
@ -330,6 +330,7 @@ const InnerSpacePanel = React.memo<IInnerSpacePanelProps>(
|
|||
);
|
||||
|
||||
const SpacePanel: React.FC = () => {
|
||||
const [dragging, setDragging] = useState(false);
|
||||
const [isPanelCollapsed, setPanelCollapsed] = useState(true);
|
||||
const ref = useRef<HTMLDivElement>(null);
|
||||
useLayoutEffect(() => {
|
||||
|
@ -344,14 +345,19 @@ const SpacePanel: React.FC = () => {
|
|||
});
|
||||
|
||||
return (
|
||||
<DragDropContext
|
||||
onDragEnd={(result) => {
|
||||
if (!result.destination) return; // dropped outside the list
|
||||
SpaceStore.instance.moveRootSpace(result.source.index, result.destination.index);
|
||||
}}
|
||||
>
|
||||
<RovingTabIndexProvider handleHomeEnd handleUpDown>
|
||||
{({ onKeyDownHandler }) => (
|
||||
<RovingTabIndexProvider handleHomeEnd handleUpDown={!dragging}>
|
||||
{({ onKeyDownHandler, onDragEndHandler }) => (
|
||||
<DragDropContext
|
||||
onDragStart={() => {
|
||||
setDragging(true);
|
||||
}}
|
||||
onDragEnd={(result) => {
|
||||
setDragging(false);
|
||||
if (!result.destination) return; // dropped outside the list
|
||||
SpaceStore.instance.moveRootSpace(result.source.index, result.destination.index);
|
||||
onDragEndHandler();
|
||||
}}
|
||||
>
|
||||
<div
|
||||
className={classNames("mx_SpacePanel", { collapsed: isPanelCollapsed })}
|
||||
onKeyDown={onKeyDownHandler}
|
||||
|
@ -395,9 +401,9 @@ const SpacePanel: React.FC = () => {
|
|||
|
||||
<QuickSettingsButton isPanelCollapsed={isPanelCollapsed} />
|
||||
</div>
|
||||
)}
|
||||
</RovingTabIndexProvider>
|
||||
</DragDropContext>
|
||||
</DragDropContext>
|
||||
)}
|
||||
</RovingTabIndexProvider>
|
||||
);
|
||||
};
|
||||
|
||||
|
|
|
@ -15,7 +15,7 @@ limitations under the License.
|
|||
*/
|
||||
|
||||
import React from "react";
|
||||
import { render, screen, fireEvent } from "@testing-library/react";
|
||||
import { render, screen, fireEvent, act } from "@testing-library/react";
|
||||
import { mocked } from "jest-mock";
|
||||
import { MatrixClient } from "matrix-js-sdk/src/matrix";
|
||||
|
||||
|
@ -24,8 +24,71 @@ import { MatrixClientPeg } from "../../../../src/MatrixClientPeg";
|
|||
import { MetaSpace, SpaceKey } from "../../../../src/stores/spaces";
|
||||
import { shouldShowComponent } from "../../../../src/customisations/helpers/UIComponents";
|
||||
import { UIComponent } from "../../../../src/settings/UIFeature";
|
||||
import { wrapInSdkContext } from "../../../test-utils";
|
||||
import { mkStubRoom, wrapInSdkContext } from "../../../test-utils";
|
||||
import { SdkContextClass } from "../../../../src/contexts/SDKContext";
|
||||
import SpaceStore from "../../../../src/stores/spaces/SpaceStore";
|
||||
import DMRoomMap from "../../../../src/utils/DMRoomMap";
|
||||
|
||||
// DND test utilities based on
|
||||
// https://github.com/colinrobertbrooks/react-beautiful-dnd-test-utils/issues/18#issuecomment-1373388693
|
||||
enum Keys {
|
||||
SPACE = 32,
|
||||
ARROW_LEFT = 37,
|
||||
ARROW_UP = 38,
|
||||
ARROW_RIGHT = 39,
|
||||
ARROW_DOWN = 40,
|
||||
}
|
||||
|
||||
enum DragDirection {
|
||||
LEFT = Keys.ARROW_LEFT,
|
||||
UP = Keys.ARROW_UP,
|
||||
RIGHT = Keys.ARROW_RIGHT,
|
||||
DOWN = Keys.ARROW_DOWN,
|
||||
}
|
||||
|
||||
// taken from https://github.com/hello-pangea/dnd/blob/main/test/unit/integration/util/controls.ts#L20
|
||||
const createTransitionEndEvent = (): Event => {
|
||||
const event = new Event("transitionend", {
|
||||
bubbles: true,
|
||||
cancelable: true,
|
||||
}) as TransitionEvent;
|
||||
|
||||
// cheating and adding property to event as
|
||||
// TransitionEvent constructor does not exist.
|
||||
// This is needed because of the following check
|
||||
// https://github.com/atlassian/react-beautiful-dnd/blob/master/src/view/draggable/draggable.jsx#L130
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
(event as any).propertyName = "transform";
|
||||
|
||||
return event;
|
||||
};
|
||||
|
||||
const pickUp = async (element: HTMLElement) => {
|
||||
fireEvent.keyDown(element, {
|
||||
keyCode: Keys.SPACE,
|
||||
});
|
||||
await screen.findByText(/You have lifted an item/i);
|
||||
|
||||
act(() => {
|
||||
jest.runOnlyPendingTimers();
|
||||
});
|
||||
};
|
||||
|
||||
const move = async (element: HTMLElement, direction: DragDirection) => {
|
||||
fireEvent.keyDown(element, {
|
||||
keyCode: direction,
|
||||
});
|
||||
await screen.findByText(/(You have moved the item | has been combined with)/i);
|
||||
};
|
||||
|
||||
const drop = async (element: HTMLElement) => {
|
||||
fireEvent.keyDown(element, {
|
||||
keyCode: Keys.SPACE,
|
||||
});
|
||||
fireEvent(element.parentElement!, createTransitionEndEvent());
|
||||
|
||||
await screen.findByText(/You have dropped the item/i);
|
||||
};
|
||||
|
||||
jest.mock("../../../../src/stores/spaces/SpaceStore", () => {
|
||||
// eslint-disable-next-line @typescript-eslint/no-var-requires
|
||||
|
@ -35,6 +98,10 @@ jest.mock("../../../../src/stores/spaces/SpaceStore", () => {
|
|||
enabledMetaSpaces: MetaSpace[] = [];
|
||||
spacePanelSpaces: string[] = [];
|
||||
activeSpace: SpaceKey = "!space1";
|
||||
getChildSpaces = () => [];
|
||||
getNotificationState = () => null;
|
||||
setActiveSpace = jest.fn();
|
||||
moveRootSpace = jest.fn();
|
||||
}
|
||||
return {
|
||||
instance: new MockSpaceStore(),
|
||||
|
@ -49,8 +116,12 @@ describe("<SpacePanel />", () => {
|
|||
const mockClient = {
|
||||
getUserId: jest.fn().mockReturnValue("@test:test"),
|
||||
getSafeUserId: jest.fn().mockReturnValue("@test:test"),
|
||||
mxcUrlToHttp: jest.fn(),
|
||||
getRoom: jest.fn(),
|
||||
isGuest: jest.fn(),
|
||||
getAccountData: jest.fn(),
|
||||
on: jest.fn(),
|
||||
removeListener: jest.fn(),
|
||||
} as unknown as MatrixClient;
|
||||
const SpacePanel = wrapInSdkContext(UnwrappedSpacePanel, SdkContextClass.instance);
|
||||
|
||||
|
@ -81,4 +152,23 @@ describe("<SpacePanel />", () => {
|
|||
screen.getByTestId("create-space-button");
|
||||
});
|
||||
});
|
||||
|
||||
it("should allow rearranging via drag and drop", async () => {
|
||||
(SpaceStore.instance.spacePanelSpaces as any) = [
|
||||
mkStubRoom("!room1:server", "Room 1", mockClient),
|
||||
mkStubRoom("!room2:server", "Room 2", mockClient),
|
||||
mkStubRoom("!room3:server", "Room 3", mockClient),
|
||||
];
|
||||
DMRoomMap.makeShared();
|
||||
jest.useFakeTimers();
|
||||
|
||||
const { getByLabelText } = render(<SpacePanel />);
|
||||
|
||||
const room1 = getByLabelText("Room 1");
|
||||
await pickUp(room1);
|
||||
await move(room1, DragDirection.DOWN);
|
||||
await drop(room1);
|
||||
|
||||
expect(SpaceStore.instance.moveRootSpace).toHaveBeenCalledWith(0, 1);
|
||||
});
|
||||
});
|
||||
|
|
Loading…
Reference in New Issue