Auto scrolling (and more!) #321

alexreardon · 2018-02-12T22:04:45Z

This PR contains:

New accessibility improvements. Closes Screen reader support #311
New auto scrolling behaviour. Closes Auto scrolling #27
New onDragUpdate hook. Closes Extra information while dragging #298 Closes Screen reader support #311
More information available in DraggableStateSnapshot and DroppableStateSnapshot. Closes Extra information while dragging #298
Maintain scroll location on drop. Adding a data-* hook to Droppables for future global styling. Closes Wrong scroll position after drop in scroll container #290

…o auto-scroll

alexreardon · 2018-02-20T09:33:50Z

docs/guides/screen-reader.md

@@ -0,0 +1,139 @@
+# Screen reader guide


this guide is still a WIP

alexreardon · 2018-02-20T09:34:32Z

src/state/action-creators.js

+  |}
+|}
+
+export const bulkPublishDimensions = (


now one big action to bulk publish dimensions. This gets around a timing issue for onDragUpdate

alexreardon · 2018-02-20T09:35:18Z

src/state/action-creators.js

@@ -469,7 +492,14 @@ export const lift = (id: DraggableId,
    }

    // will communicate with the marshal to start requesting dimensions
-    dispatch(requestDimensions(id));
+    const scrollOptions: ScrollOptions = {
+      shouldPublishImmediately: autoScrollMode === 'JUMP',


we have a different Droppable scroll strategy for JUMP auto scrolling

alexreardon · 2018-02-20T09:39:53Z

src/state/auto-scroller/is-too-big-to-auto-scroll.js

+// @flow
+import type { Area } from '../../types';
+
+export default (frame: Area, subject: Area): boolean =>


we do not auto scroll if the item is too big!

alexreardon · 2018-02-20T09:40:14Z

src/state/axis.js

@@ -4,7 +4,7 @@ import type { HorizontalAxis, VerticalAxis } from '../types';
 export const vertical: VerticalAxis = {
  direction: 'vertical',
  line: 'y',
-  crossLine: 'x',
+  crossAxisLine: 'x',


renamed property to bring it in line with other crossAxis properties

alexreardon · 2018-02-20T09:46:41Z

src/state/reducer.js

+        ...existing,
+        current: {
+          ...existing.current,
+          hasCompletedFirstBulkPublish: true,


important new flag to prevent incorrect onDragUpdate announcements

alexreardon · 2018-02-20T09:47:16Z

src/state/reducer.js

+    }
+
+    const impact: ?DragImpact = (() => {
+      // we do not want to recalculate the initial impact until the first bulk publish is finished


boom. preventing incorrect onDragUpdate calls right here

alexreardon · 2018-02-20T09:47:43Z

src/state/reducer.js

      console.error('cannot move with window scrolling if no current drag');
      return clean();
    }

+    if (isEqual(windowScroll, drag.current.windowScroll)) {


we can exit a little early here

alexreardon · 2018-02-20T09:48:16Z

src/state/visibility/is-visible.js

+
+const origin: Position = { x: 0, y: 0 };
+
+const isVisible = ({


improved visibility checks

alexreardon · 2018-02-20T09:48:51Z

src/types.js

  // may not have any destination (drag to nowhere)
  destination: ?DraggableLocation,
 |}

+export type DropReason = 'DROP' | 'CANCEL';


we publish this reason to allow for improved screen reader messaging

jaredcrowe · 2018-02-22T00:07:19Z

src/state/auto-scroller/fluid-scroller.js

+  }
+
+  // Only too big on one axis
+  // Exclude the axis that we cannot scroll on


Yeah!! So good

jaredcrowe · 2018-02-22T00:13:31Z

src/state/auto-scroller/fluid-scroller.js

+    }
+
+    // We know this has a closestScrollable
+    const closestScrollable: ClosestScrollable = (droppable.viewport.closestScrollable : any);


might be able to change line 285 to if (!droppable || !droppable.viewport.closestScrollable) { to avoid having to cast through any here?

jaredcrowe · 2018-02-22T00:31:21Z

src/state/auto-scroller/jump-scroller.js

+  };
+
+  return jumpScroller;
+};


I feel like the logic in this file is overly complicated. Can't it be simplified to something like this:

We have the scroll request.

We scroll the droppable as much as we can to fulfill the request and subtract however far it could move from the request. This gives us a remainder A.

We scroll the window as much as we can to fulfill remainder A and subtract that again to get remainder B.

We move the draggable by remainder B. Now we're done.

If the remainder is zero at the end of any step we can bail early.

I think the specific checks to see if the droppable can scroll, or absorb the entire jump, or if the window can scroll, or absorb the entire jump etc. are redundant. If the window or the droppable can't scroll then we subtract zero from the request and move to the next step. If they can absorb all of the jump then the remainder is zero so nothing will be done in the subsequent steps.

I think that's basically what you're doing, but I don't know why case 1. and case 2. in the code are being treated as different paths?

jaredcrowe · 2018-02-22T00:34:52Z

src/state/get-max-scroll.js

+
+  return adjustedMaxScroll;
+
+  // return maxScroll;


jaredcrowe · 2018-02-22T00:37:16Z

src/view/announcer/announcer.js

+
+export default (): Announcer => {
+  const id: string = `react-beautiful-dnd-announcement-${count++}`;
+  // const id: string = 'react-beautiful-dnd-announcement';


alexreardon added 30 commits January 17, 2018 08:48

reshaping droppable dimension for clarity

31f36aa

initial

3d9823f

basic scroll window down version

26405fd

wip

9fb48cb

fiddling with auto scroll for touch

77e8686

wip

5eff709

prototype of window scroll working

317ed80

fiddling

e2825b6

wip

2d99b9d

detecting frame over

9bf1100

Merge branch 'master' of github.com:atlassian/react-beautiful-dnd int…

316aacc

…o auto-scroll

wip

18e4a1a

as good as we can do

db199bd

uncommenting droppable code

a8993ef

scroll handover

47962de

minor

2742d41

adding DroppableProps type

97cd95b

fixing incorrect droppable offset due to scroll on drop

aa20b59

adding comment

c5cef97

progress

5338ad4

more is too big logic

3003c72

correcting keyboard window scrolling

6a97c5b

progress

b9946f9

giving more information for scroll jumping

a2858bb

more fiddling

ac909e9

in home list seems to be working

db45ca6

in home list is looking niace

f538ced

correcting droppable scroll

a1e8ab1

new droppable dimension type

2e0f1f1

responding to type refactor

6f87401

alexreardon mentioned this pull request Feb 20, 2018

update to babel-preset-env #325

Closed

alexreardon added 4 commits February 20, 2018 15:13

more tests

249f6cb

more tests. more green

2c8d1e2

removing comment

c4ec91c

Merge remote-tracking branch 'origin/master' into auto-scroll

549723a

alexreardon requested a review from jaredcrowe February 20, 2018 05:20

alexreardon self-assigned this Feb 20, 2018

updating docs

06168c3

alexreardon commented Feb 20, 2018

View reviewed changes

alexreardon added 7 commits February 21, 2018 13:30

improved logic for opting out of auto scrolling

d9bb350

attempt 1

42191ad

removing is too big logic from jump scroller (it is not needed)

0b01ac0

new improved experience for big items

6010f90

more tests

bd04af7

removing unneeded jump scroller tests

8b68af5

shuffling code around

fb8e3bc

jaredcrowe reviewed Feb 22, 2018

View reviewed changes

improvements to docs and screen-reader guide

6fde4d2

jaredcrowe reviewed Feb 22, 2018

View reviewed changes

alexreardon added 5 commits February 22, 2018 14:16

simplified jump-scroll logic

da2b9ce

fixing comment

f5c5ab1

minor changes in response to PR feedback

e641884

removing comment

c8a1146

improving comment

9763762

jaredcrowe approved these changes Feb 22, 2018

View reviewed changes

alexreardon merged commit 65f115f into master Feb 22, 2018

This was referenced Feb 23, 2018

Maintain scroll position post-drop #291

Closed

Ensure that library works well with screen readers #7

Closed

alexreardon deleted the auto-scroll branch March 23, 2018 02:04

gitJoe42 mentioned this pull request Jul 31, 2019

Uncaught TypeError: $.memo is not a function at react-beautiful-dnd.cjs.js #1416

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Auto scrolling (and more!) #321

Auto scrolling (and more!) #321

alexreardon commented Feb 12, 2018 •

edited

Loading

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

alexreardon Feb 20, 2018

jaredcrowe Feb 22, 2018

alexreardon Feb 22, 2018

jaredcrowe Feb 22, 2018

alexreardon Feb 22, 2018

jaredcrowe Feb 22, 2018

alexreardon Feb 22, 2018

jaredcrowe Feb 22, 2018

alexreardon Feb 22, 2018 •

edited

Loading

jaredcrowe Feb 22, 2018

alexreardon Feb 22, 2018


		const origin: Position = { x: 0, y: 0 };

		const isVisible = ({

Auto scrolling (and more!) #321

Auto scrolling (and more!) #321

Conversation

alexreardon commented Feb 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexreardon Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexreardon commented Feb 12, 2018 •

edited

Loading

alexreardon Feb 22, 2018 •

edited

Loading