search

silverstripe / silverstripe-elemental

Create pages in Silverstripe CMS using content blocks
http://dna.co.nz
BSD 3-Clause "New" or "Revised" License
110 stars 115 forks source link

SPIKE: Keyboard accessible reordering of blocks #455

Closed brynwhyman closed 5 years ago

brynwhyman commented 6 years ago

It is currently not possible on master to re-order blocks on a page with keyboard actions. We should investigate the best way to implement functionality that provides keyboard access to reordering functionality.

Drag and drop will be implemented via #285 but https://github.com/react-dnd/react-dnd doesn't provide accessible functionality out of the box.

Ideas (in likely priority) ~1. Ensure inline adding of blocks can be tabbed to:~ split to #627 image

  1. Investigate if accessibility can be built on top of react-dnd/react-dnd
  2. Investigate recent accessibility improvements in https://github.com/atlassian/react-beautiful-dnd although additional development will be expected.
  3. Look into a 'move block up/down' function to +1/-1 the order of the block on the page
clarkepaul commented 6 years ago

Seems like a decent article on this matter here, and demo.

ScopeyNZ commented 6 years ago

Here's the beautiful DND examples: https://react-beautiful-dnd.netlify.com/?selectedKind=single%20vertical%20list&selectedStory=basic&full=0&addons=1&stories=1&panelRight=0&addonPanel=storybook%2Factions%2Factions-panel

The DND PR is good to go - would be interested to look at a refactor to this library but it might mean starting from scratch.

newleeland commented 5 years ago

Don't think this requires designs, react-beautiful-dnd seems to have built-in drag-n drop for screen readers. https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/screen-reader.md

NightJar commented 5 years ago

I'm not an expert on accessibility, and for this reason I think transferring to the new react-beautiful-dnd library would be a superior option to attempting to self-engineer (self as in team) a retrofit of accessibility hints, controls, and usage flows to the current solution. So the outcome of this investigation has been based/biased around this. Both libraries are well maintained, I don't see one as being preferable to the other on this criterion alone.

react-dnd has it's roots in silverstripe/admin so we'd have to update that to take advantage of a new library. To my knowledge we're the only ones consuming this export - but I did not attempt to check this assumption.

We are using DragSource and DropTarget from the "Legacy decorator API" in react-dnd rather than the newer useDrag and useDrop, where I personally find the legacy API a little easier to follow - plus this is more relatable to the Draggable and Droppable API provided by react-beautiful-dnd. This is a kind of win for an easy port I think - the implementation details are mostly transferrable; The wrappers provide props to child components that update with dragging state. The details change, but the concept remains the same.

react-dnd is activated via lib/withDragDropContext in admin, which is a simple wrapper around exporting the library's wrapper with a backend attached (the HTML5 backend, specifically). This is an internal that could hopefully be updated without need for making a new release of webpack config.

The react-beautiful-dnd library supplies an element of the same name DragDropContext for the creation of the drag and drop area, however takes different properties - the important difference being that the hooks for updating before, during and after a drag are contained here, rather than on the Draggable/DragSource and Droppable/DropTarget as per react-dnd.

Other properties living on Draggable/DragSource and Droppable/DropTarget seem mostly transferrable, although they do change name (e.g. canDrop becomes isDropDisabled). I don't think this would be a huge burden at all.

Switching library here would also have a knock on affect, in that react-beautiful-dnd supports dragging between lists, which is asked about every so often in the community, by people adding more than one ElementalArea to their objects.

I also think the combining feature would provide a head start for #332 - allowing existing elements t be dragged in and out of a Layout Block.

Implementation intro: https://egghead.io/lessons/react-reorder-a-list-with-react-beautiful-dnd

TL;DR react-beautiful-dnd :heavy_check_mark: More functionality, less maintenance burden, and cursory future proofing provided out of the box for a very similar API - with hopefully mostly transferrable implementation concepts. Minor concerns around how this impacts silverstripe/admin on a greater scale.

ScopeyNZ commented 5 years ago

There's still a couple of oddities about our use case that might need to be considered.

We have drag areas within drag areas - that's why admin exposes a HOC (withDragDropContext). ReactDND has an issue with two separate contexts being created so they need to share the same one. Not sure if this is going to be an issue if we have ReactDND inside a React-Beautiful-DND? For reference this is when you have an UploadField within an in-line edit form.

Our UI for dragging is a little custom. We leave an indication of the current position within our list of blocks and we insert a line to show where the element will drop. I assume that this will probably have to change with ReactDND? It forces you down the route of showing where it'll land and not indicating the existing location...

Currently we use another "power feature" (of sorts) with ReactDND - the ability to have custom drag previews for our elements. We do this because the in-line edit forms can be pretty massive to be dragging up and down the screen. I remember seeing an issue saying this wasn't possible with ReactDND - although you could re-render the component by using some isDragging prop. We might need to do some POC work to see if this works though as I have a feeling this could cause issues when the draggable area suddenly changes size on you.

Playing a little bit of a devils' advocate here but I think we need to be sure that we're not going to create a big project for ourselves by diving into "switch out the DND library" - especially when there's this quote on the beautiful DND page:

However, it does not provide the breadth of functionality offered by react-dnd. So react-beautiful-dnd might not be for you depending on what your use case is.

robbieaverill commented 5 years ago

I'm not overly familiar with the implementation of the drag and drop library, but I'd tend to agree with @ScopeyNZ's comments - replacing the library seems to me like it'd be more work than implementing keyboard accessibility in the library we already have.

I can't find anything obvious in the docs or issue tracker for it relating to being able to control the drag/drop events with keyboard buttons, however I think that we could either add it ourselves, or simply add a "Move down" and "Move up" option to the actions dropdown to compensate for it if that's too difficult.

Cheddam commented 5 years ago

I wouldn't be too quick to assume creating a good a11y experience on our own will be easier than switching library, particularly when react-dnd makes no mention of it in their documentation whilst react-beautiful-dnd goes step by step through the different audible descriptions it provides.

It was discussed at standup that we've missed another option, though: Using standard buttons or options in the Actions panel to enable shifting blocks up or down for keyboard access / screen readers. This would be less efficient from a usability perspective, but easier to implement well. @newleeland might have some designs for this?

EDIT: I realise now that @robbieaverill mentioned the button approach above.

NightJar commented 5 years ago

There's still a couple of oddities about our use case that might need to be considered.

We have drag areas within drag areas - that's why admin exposes a HOC (withDragDropContext). ReactDND has an issue with two separate contexts being created so they need to share the same one. Not sure if this is going to be an issue if we have ReactDND inside a React-Beautiful-DND? For reference this is when you have an UploadField within an in-line edit form.

This would pose a challenge, and I did not look into it I'm afraid (as I didn't realise this complexity at the time).

Our UI for dragging is a little custom. We leave an indication of the current position within our list of blocks and we insert a line to show where the element will drop. I assume that this will probably have to change with ReactDND? It forces you down the route of showing where it'll land and not indicating the existing location...

I don't think this is true. I note the difference between a DragLayer (react-dnd) and not having one, but I believe this is replaced by 'Responder' properties on the context (https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/responders.md#ondragupdate-optional), which I think can still achieve the same thing (showing drop indicators, etc.)

Currently we use another "power feature" (of sorts) with ReactDND - the ability to have custom drag previews for our elements. We do this because the in-line edit forms can be pretty massive to be dragging up and down the screen. I remember seeing an issue saying this wasn't possible with ReactDND - although you could re-render the component by using some isDragging prop. We might need to do some POC work to see if this works though as I have a feeling this could cause issues when the draggable area suddenly changes size on you.

This may be true. I did not see any indication of the functionality for horseface to be shown on a drag. I think it would probably be possible, but as with the link above about the responders, the update function docs do warn: It is important that you not do too much work as a result of this function as it will slow down the drag.

Playing a little bit of a devils' advocate here but I think we need to be sure that we're not going to create a big project for ourselves by diving into "switch out the DND library" - especially when there's this quote on the beautiful DND page:

However, it does not provide the breadth of functionality offered by react-dnd. So react-beautiful-dnd might not be for you depending on what your use case is.

Reference: Not for everyone ✌️ This quote is in the context that react-beautiful-dnd is build specifically for ordering lists, where react-dnd is more generic and one should consider their use case. I did, an ElementalEditor is made up of a single vertical list of elements. On the other hand the example for react-dnd is dragging a knight around a chessboard - not quite a single list.

I can't find anything obvious in the docs or issue tracker for it relating to being able to control the drag/drop events with keyboard buttons, however I think that we could either add it ourselves, or simply add a "Move down" and "Move up" option to the actions dropdown to compensate for it if that's too difficult.

I agree with @Cheddam on this - I would not presume that creating good a11y interfaces should be simple, otherwise I do not think there would be an industry specialisation around this. Keeping in mind that anything we build will be ours to maintain, I think the ability to 'outsource' or offload as much of that as possible is a good choice.

Adding 'move up' and 'move down' options to a menu seems only a viable option if the menu were to stay open and maintain focus between move operations, so one could move an item up and down in succession quickly, rather than tedious menu interactions each time.

On this though I have forgotten to mention that I also considered that we have a drag handle - although we don't use it. Selecting a block currently and activating it will open or collapse the inline edit form - this will need to change, or we would have to activate the drag only when the handle is active - which would mean making that 'tab-able' - but would also be a tricky consideration as it is the entire element header that is the mouse activated drag handle. This will be a challenge in either library. Which ultimately puts more weight on the "buttons" approach.

robbieaverill commented 5 years ago

Sorry, I wasn't trying to suggest that it would be simple, but rather that we should weigh up the effort versus reward. Here's where I see the options for this issue now:

Option 1: replace react-dnd with react-beautiful-dnd

React with 🎉 to vote for option 1

Pros:

Cons:

Option 2: keep react-dnd and implement out own keyboard accessibility events

React with ❤️ to vote for option 2

Pros:

Cons:

Option 3: add new options to the more actions menu to move elements

React with 🚀 to vote for option 3

Example:

image.png

Pros:

Cons:

Option 4: add tab key accessible "up" and "down" options in the blue "add new element" bar somewhere on the element header

React with 👀 to vote for option 4

Example:

image.png

(Except use icons instead of text)

Pros:

Cons:

@silverstripe/creative-commoners please vote on this so we can agree on the way forward, and then close this spike =) cc @silverstripeux

Cheddam commented 5 years ago

For Option 4, I don't think the up / down could live in the "add new element" area - the ownership of the buttons would be unclear. Instead, I'd see them living next to the Actions menu. This would require more tabbing to navigate to the Actions menu, making the interaction less convenient - but accessible controls are far more important than convenient ones.

Given the reluctance to switch, and the fact that react-dnd is used elsewhere in SilverStripe, I'm opting to vote for Option 2 at this stage - on the basis that we actually take the time to make this work well. I'd also personally be happy with, and am voting for, Option 4 (with the caveats above).

clarkepaul commented 5 years ago

My suggestion would be to make an area near the drag icon tab-able with the arrow icon actions to move the item up or down. The drag handle would help to provide context as to what the actions do. The options dropdown is going to get overloaded with more actions soon or through a project's development (duplicate, move, other tabs).

Pasted_Image_23_07_19__11_11_AM
NightJar commented 5 years ago

Also to be clear: Option 4 is not viable; This menu (or this hover bar rather) is not accessible via keyboard navigation - it only appears on hover - which is of course a mouse event. updated to reflect edits top options list, option 4 has since been changed to match the suggestion immediately above

robbieaverill commented 5 years ago

That sounds like a fixable bug to me :-)

NightJar commented 5 years ago

I also don't agree that it's a good solution (but that's what voting is for). I like Paul's suggestion above better :)

robbieaverill commented 5 years ago

Yeah I agree- feel fre esto change option 4 to Paul’s suggestion. My vote would stay the same

NightJar commented 5 years ago

OK, I did that, and added my vote for it :)

ScopeyNZ commented 5 years ago

I voted ❤️ 🚀 and 👀 . I think that ❤️ is a bit of a pipe-dream but I like the idea of doing both 🚀 and 👀 . I even think 🚀 will be minimal effort.

NightJar commented 5 years ago

Neat, so the outcome of this SPIKE is to implement "move up" and "move down" buttons on the ElementHeader, with a stretch goal of having them available as actions in the more options menu also. As a side note the drag handle will need to be permanently visible when an element is focussed at least, probably always, at best.

I'll create a new issue.

698