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

ENH Improve keyboard support #200

Merged
merged 1 commit into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion client/dist/styles/bundle.css

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion client/lang/en.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ if (typeof(ss) === 'undefined' || typeof(ss.i18n) === 'undefined') {
"LinkField.DELETE_SUCCESS": "Deleted link",
"LinkField.ARCHIVE_ERROR": "Failed to archive link",
"LinkField.DELETE_ERROR": "Failed to delete link",
"LinkField.ADD_LINK": "Add Link",
"LinkField.ADD_LINK": "Add link",
"LinkField.EDIT_LINK": "Edit link",
"LinkField.ARCHIVE": "Archive",
"LinkField.DELETE": "Delete",
"LinkField.LINK_DRAFT_TITLE": "Link has draft changes",
Expand Down
3 changes: 2 additions & 1 deletion client/lang/src/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@
"LinkField.DELETE_SUCCESS": "Deleted link",
"LinkField.ARCHIVE_ERROR": "Failed to archive link",
"LinkField.DELETE_ERROR": "Failed to delete link",
"LinkField.ADD_LINK": "Add Link",
"LinkField.EDIT_LINK": "Edit link",
"LinkField.ADD_LINK": "Add link",
"LinkField.ARCHIVE": "Archive",
"LinkField.DELETE": "Delete",
"LinkField.LINK_DRAFT_TITLE": "Link has draft changes",
Expand Down
162 changes: 145 additions & 17 deletions client/src/components/LinkField/LinkField.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
/* eslint-disable */
import React, { useState, useEffect, createContext } from 'react';
import React, { useState, useEffect, useRef, createContext } from 'react';
import { bindActionCreators, compose } from 'redux';
import { connect } from 'react-redux';
import { DndContext, closestCenter, PointerSensor, useSensor, useSensors } from '@dnd-kit/core';
import {
closestCenter,
DndContext,
KeyboardCode,
KeyboardSensor,
PointerSensor,
useSensor,
useSensors
} from '@dnd-kit/core';
import { arrayMove, SortableContext, verticalListSortingStrategy } from '@dnd-kit/sortable';
import { restrictToVerticalAxis, restrictToParentElement } from '@dnd-kit/modifiers';
import { injectGraphql } from 'lib/Injector';
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated refactoring to remove an unused import

import fieldHolder from 'components/FieldHolder/FieldHolder';
import LinkPicker from 'components/LinkPicker/LinkPicker';
import LinkPickerTitle from 'components/LinkPicker/LinkPickerTitle';
Expand Down Expand Up @@ -42,7 +49,6 @@ const section = 'SilverStripe\\LinkField\\Controllers\\LinkFieldController';
const LinkField = ({
value = null,
onChange,
onNonPublishedVersionedState,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated refactoring to remove a non-existant prop

types = {},
actions,
isMulti = false,
Expand All @@ -56,6 +62,10 @@ const LinkField = ({
}) => {
const [data, setData] = useState({});
const [editingID, setEditingID] = useState(0);
const [isKeyboardEditing, setIsKeyboardEditing] = useState(false);
const [focusOnID, setFocusOnID] = useState(0);
const [focusOnNewLinkWhenClosed, setFocusOnNewLinkWhenClosed] = useState(false);
const [focusOnNewLink, setFocusOnNewLink] = useState(false);
const [loading, setLoading] = useState(false);
const [forceFetch, setForceFetch] = useState(0);
const [isSorting, setIsSorting] = useState(false);
Expand All @@ -66,6 +76,58 @@ const LinkField = ({
activationConstraint: {
distance: 10
}
}),
useSensor(KeyboardSensor, {
coordinateGetter: (event, args) => {
event.preventDefault();
const { active, over, droppableContainers } = args.context;
if (!active || !active.data || !active.data.current) {
return;
}
const items = active.data.current.sortable.items;
const overId = over ? over.id : active.id;
const overIndex = items.indexOf(overId);
const activeIndex = items.indexOf(active.id);
const directionUp = -1;
const directionDown = 1;
let nextIndex = overIndex;
let direction = directionDown;
switch (event.code) {
case KeyboardCode.Down:
case KeyboardCode.Right:
nextIndex = Math.min(overIndex + 1, items.length - 1);
break;
case KeyboardCode.Up:
case KeyboardCode.Left:
nextIndex = Math.max(0, overIndex - 1);
direction = directionUp;
break;
default:
return;
}
if (overIndex === nextIndex) {
return;
}
const sortedItems = arrayMove(items, activeIndex, overIndex);
const currentNodeIdAtNextIndex = sortedItems[nextIndex];
if (!droppableContainers.has(currentNodeIdAtNextIndex)) {
return;
}
const activeNode = droppableContainers.get(active.id).node.current;
if (!droppableContainers.has(active.id)) {
return;
}
const newNode = droppableContainers.get(currentNodeIdAtNextIndex).node.current;
const activeRect = activeNode.getBoundingClientRect();
const newRect = newNode.getBoundingClientRect();
const offset = direction === directionDown
? newRect.top - activeRect.bottom
: activeRect.top - newRect.bottom;
return {
x: 0,
y: activeRect.top + direction * (newRect.height + offset),
};
}
})
);

Expand Down Expand Up @@ -107,34 +169,96 @@ const LinkField = ({
}
}, [editingID, value && value.length, forceFetch]);

// Create refs for each LinkPickerTitle button so they can be focused when the editing modal is closed via keyboard
let refCount = 0;
const linkButtonRefs = []
for (const linkID of linkIDs) {
linkButtonRefs[linkID] = useRef(null);
refCount++;
}
// Ensure the exact same number of hooks are called on every render
// If this this isn't done then a react error will be thrown when a link is deleted
while (refCount < 256) {
useRef(null);
refCount++;
}

// Focus on the LinkPickerTitle edit button when the editing modal is closed via keyboard
// or focus on newly created link for single (has_one) linkfield
useEffect(() => {
sabina-talipova marked this conversation as resolved.
Show resolved Hide resolved
if ((!focusOnID && !focusOnNewLink) || loading) {
return;
}
let c = 0;
const interval = setInterval(() => {
if (focusOnID && linkButtonRefs[focusOnID].current) {
// Multi linkfield
linkButtonRefs[focusOnID].current.focus();
clearInterval(interval);
} else if (focusOnNewLink) {
// Non-multi linkfield
if (linkIDs.length === 0) {
// User opened modal but did exited instead of saving
clearInterval(interval);
} else {
// User opened modal and created a new link
const linkID = linkIDs[0];
if (linkButtonRefs[linkID].current) {
linkButtonRefs[linkID].current.focus();
clearInterval(interval);
}
}
}
// Safety check
if (++c >= 50) {
clearInterval(interval);
}
}, 50);
setFocusOnID(0);
setFocusOnNewLink(false);
}, [focusOnID, focusOnNewLink, loading]);

/**
* Unset the editing ID when the editing modal is closed
* If using keyboard, focus on button used to open the modal
*/
const onModalClosed = () => {
const handleModalClosed = () => {
if (isKeyboardEditing) {
setIsKeyboardEditing(false);
if (editingID) {
setFocusOnID(editingID);
} else if (focusOnNewLinkWhenClosed) {
setFocusOnNewLink(true);
}
}
setEditingID(0);
setFocusOnNewLinkWhenClosed(false);
};

/**
* Update the component when the modal successfully saves a link
*/
const onModalSuccess = (value) => {
// update component state
setEditingID(0);

const handleModalSuccess = (value) => {
handleModalClosed();
const ids = [...linkIDs];
if (!ids.includes(value)) {
ids.push(value);
}

// Update value in the underlying <input> form field
// so that the Page (or other parent DataObject) gets the Link relation set.
// Also likely required in react context for dirty form state, etc.
onChange(isMulti ? ids : ids[0]);

// success toast
actions.toasts.success(i18n._t('LinkField.SAVE_SUCCESS', 'Saved link'));
}

const handleLinkPickerKeyDownEdit = () => {
if (!isMulti) {
setFocusOnNewLinkWhenClosed(true);
}
setIsKeyboardEditing(true);
}

/**
* Update the component when the 'Delete' button in the LinkPicker is clicked
*/
Expand Down Expand Up @@ -219,7 +343,6 @@ const LinkField = ({
*/
const renderLinks = () => {
const links = [];

for (let i = 0; i < linkIDs.length; i++) {
const linkID = linkIDs[i];
// Only render items we have data for
Expand All @@ -238,6 +361,7 @@ const LinkField = ({
typeIcon={type.icon}
onDelete={handleDelete}
onClick={() => { setEditingID(linkID); }}
onButtonKeyDownEdit={() => setIsKeyboardEditing(true)}
onUnpublishedVersionedState={handleUnpublishedVersionedState}
canDelete={data[linkID]?.canDelete ? true : false}
isMulti={isMulti}
Expand All @@ -247,14 +371,15 @@ const LinkField = ({
canCreate={canCreate}
readonly={readonly}
disabled={disabled}
buttonRef={linkButtonRefs[linkID]}
/>);
}
return links;
};

const sortableLinks = () => {
if (isMulti && !readonly && !disabled) {
return <div className={linksClassName}>
return <div className={linksClassName} onBlur={() => setIsSorting(false)}>
<DndContext modifiers={[restrictToVerticalAxis, restrictToParentElement]}
sensors={sensors}
collisionDetection={closestCenter}
Expand Down Expand Up @@ -327,21 +452,24 @@ const LinkField = ({
{ saveRecordFirst && <div className="link-field__save-record-first">{saveRecordFirstText}</div>}
{ loading && !isSorting && !saveRecordFirst && <Loading containerClass="link-field__loading"/> }
{ renderPicker && <LinkPicker
onModalSuccess={onModalSuccess}
onModalClosed={onModalClosed}
onModalSuccess={handleModalSuccess}
onModalClosed={handleModalClosed}
types={types}
canCreate={canCreate}
readonly={readonly}
disabled={disabled}
onKeyDownEdit={handleLinkPickerKeyDownEdit}
isKeyboardEditing={isKeyboardEditing}
/> }
{sortableLinks()}
{ renderModal && <LinkModalContainer
types={types}
typeKey={data[editingID]?.typeKey}
isOpen={Boolean(editingID)}
onSuccess={onModalSuccess}
onClosed={onModalClosed}
onSuccess={handleModalSuccess}
onClosed={handleModalClosed}
linkID={editingID}
autoFocus={isKeyboardEditing}
/>
}
</div>
Expand Down
69 changes: 66 additions & 3 deletions client/src/components/LinkField/tests/LinkField-test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* global jest, test */
/* global jest, test, expect, document */
import React from 'react';
import { render, act } from '@testing-library/react';
import { render, act, screen } from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom';
import { Component as LinkField } from '../LinkField';

let doResolve;
Expand Down Expand Up @@ -28,7 +30,16 @@ function makeProps(obj = {}) {
return {
value: 123,
onChange: () => {},
types: {},
types: {
mylink: {
key: 'mylink',
title: 'My Link',
handlerName: 'FormBuilderModal',
priority: 100,
icon: 'font-icon-link',
allowed: true
}
},
actions: {
toasts: {
success: () => {},
Expand All @@ -46,6 +57,58 @@ function makeProps(obj = {}) {
};
}

test('LinkField tab order', async () => {
const user = userEvent.setup();
const { container } = render(<LinkField {...makeProps({
isMulti: true,
value: [123, 456],
})}
/>);

await doResolve({ json: () => ({
123: {
Title: 'First title',
Sort: 1,
typeKey: 'mylink',
},
456: {
Title: 'Second title',
Sort: 2,
typeKey: 'mylink',
},
}) });
await screen.findByText('First title');

expect(Array.from(container.querySelectorAll('.link-picker__title-text')).map(el => el.innerHTML))
.toStrictEqual(['First title', 'Second title']);

const linkPicker123 = container.querySelector('#link-picker__link-123');
const button123 = linkPicker123.querySelector('.link-picker__button');
const dragHandle123 = linkPicker123.querySelector('.link-picker__drag-handle');
const linkPicker456 = container.querySelector('#link-picker__link-456');
const button456 = linkPicker456.querySelector('.link-picker__button');
const dragHandle456 = linkPicker456.querySelector('.link-picker__drag-handle');

// Focus starts on document <body>
expect(container.parentNode).toHaveFocus();
await user.tab();
expect(container.querySelector('.link-picker__menu-toggle')).toHaveFocus();
// note need to tab twice because jest will focus on the .dropdown-item, however in a real browser
// this doesn't happen because it will have a display of none at this point
await user.tab();
await user.tab();
expect(dragHandle123).toHaveFocus();
await user.tab();
expect(button123).toHaveFocus();
await user.tab();
expect(dragHandle456).toHaveFocus();
await user.tab();
expect(button456).toHaveFocus();

// Note that we cannot test keyboard sorting with up + down keys in jest because jsdom does not have a layout engine
// e.g. el.getBoundingClientRect() will always return 0,0,0,0
});

test('LinkField will render save-record-first div if ownerID is 0', async () => {
const { container } = render(<LinkField {...makeProps({
ownerID: 0
Expand Down
Loading
Loading