Cancel drag when no visibility #83 #396

grady-lad · 2018-03-21T21:19:30Z

Fixes #83

Quick Demo

alexreardon · 2018-03-21T23:01:18Z

src/view/drag-handle/util/create-visibility-event.js

+};
+
+export default (cancel: Function = () => {}) : EventBinding => {
+  if (typeof window !== 'undefined') {


When can this function be called when window is not around?

In the integration tests for SSR
Within test/unit/integration/server-side-rendering.spec.js the following tests fail

should render identical content when resetting context between renders

should support rendering to static markup

should support rendering to a string

alexreardon · 2018-03-21T23:07:45Z

src/view/drag-handle/util/create-visibility-event.js

+
+const getVisibiltyEvent = () : ?{hidden:string, name:string} => {
+  const prefixes = ['ms', 'webkit', 'moz', 'o'];
+  let browserPrefix;


I think we can just use a find here (we are already using .find around the place). Also, I think we should check for the default first:

// using memoizeOne so that we do not recompute this function after initial computation const getVisibilityEvent = memoizeOne() : ?Pair => { // non prefixed event is supported if('hidden' in document) { return { hidden: 'hidden', name: 'visibilitychange', } const prefix: ?string = ['ms', 'webkit', 'moz', 'o'].find((prefix: string): boolean => `${prefix}Hidden` in document); if(!prefix) { return null; } return { hidden: `${prefix}Hidden`, name: `${prefix}visibilitychange`, } } })

const defaultPair: Pair = { hidden: 'hidden', name: 'visibilitychange', } // using memoizeOne so that we do not recompute this function after initial computation const getVisibilityEvent = memoizeOne() : ?Pair => { // non prefixed event is supported if('hidden' in document) { return defaultPair; const prefix: ?string = ['ms', 'webkit', 'moz', 'o'].find((prefix: string): boolean => `${prefix}Hidden` in document); // if no prefixed event is supported - simply return the defaultPair. It will not cause any errors to bind to this event - it just won't do anything on visibility change if(!prefix) { return defaultPair; } return { hidden: `${prefix}Hidden`, name: `${prefix}visibilitychange`, } } })

alexreardon · 2018-03-21T23:08:35Z

test/unit/view/drag-handle.spec.js

@@ -922,6 +922,23 @@ describe('drag handle', () => {
        expect(event.defaultPrevented).toBe(false);
      });

+      it('should cancel when the window is not visibile', () => {


can this test please be moved to the shared 'control' section at the bottom so that each sensor type can be tested against this functionality?

Sure thing, Which section? Is it the interactive elements section?

alexreardon · 2018-03-21T23:12:35Z

test/setup.js

@@ -22,8 +22,10 @@ if (typeof window !== 'undefined') {
 if (typeof document !== 'undefined' && typeof window !== 'undefined') {
  document.documentElement.clientWidth = window.innerWidth;
  document.documentElement.clientHeight = window.innerHeight;
+  // So that we can test window visibility within tests.
+  document.webkitvisibilitychange = () => {};


given that we check 'hidden' first we should move to the non prefixed one

alexreardon · 2018-03-21T23:13:10Z

test/setup.js

@@ -22,8 +22,10 @@ if (typeof window !== 'undefined') {
 if (typeof document !== 'undefined' && typeof window !== 'undefined') {
  document.documentElement.clientWidth = window.innerWidth;
  document.documentElement.clientHeight = window.innerHeight;
+  // So that we can test window visibility within tests.
+  document.webkitvisibilitychange = () => {};
+  document.webkitHidden = true;


I think it makes sense to have the default has

document.hidden = false;

rather than 'true'.

@alexreardon I am a little stuck about mocking the visibility event for the unit tests.

The document object that is used for the unit tests has a property hidden but no visibilitychange function. So within test/setup.js I added the visibilitychange function to the document object and within that function tried set document.hidden = true.

I presumed that would mock the window being hidden but the function does not get called.

So I am wondering does anyone have any suggestions how to mock this?

You can trigger your own events manually. See user-input-util

export const dispatchWindowEvent = (eventName: string, options?: Object = {}): Event => { const event: Event = document.createEvent('Event'); event.initEvent(eventName, true, true); // override properties on the event itself Object.assign(event, options); window.dispatchEvent(event); return event; };

alexreardon · 2018-03-21T23:38:17Z

Thanks for getting into this @grady-lad !!!

alexreardon · 2018-03-28T03:27:54Z

This is looking good

alexreardon · 2018-03-28T03:28:27Z

However, it looks like a test is failing in CI. Did you need some help with it?

grady-lad · 2018-03-28T16:23:33Z

Hey @alexreardon I just noticed.
Yeah that would be great! I am pretty new to flow and haven't seemed to figure out the error.

alexreardon · 2018-03-29T06:20:51Z

I will try to take a look early next week. Happy easter!

alexreardon · 2018-04-03T07:26:38Z

Sorry for the delay on this one. We got totally blindsided by #413. Once things settle a bit I will take another look at this

grady-lad · 2018-04-04T20:14:40Z

@alexreardon no problems, take your time.

alexreardon · 2018-04-10T07:13:03Z

I have updated the PR @grady-lad

grady-lad · 2018-04-10T19:12:08Z

Hey @alexreardon I just seen that changes, thanks for the help 😄. Do you need anything else from my side to close this out?

alexreardon · 2018-04-10T23:29:09Z

src/view/drag-handle/sensor/create-keyboard-sensor.js

@@ -222,6 +223,11 @@ export default ({
      eventName: 'scroll',
      fn: callbacks.onWindowScroll,
    },
+    // Cancel on page visibility change


I was able to simply the solution.

We do not need to know what the visibility of the page is when the even fires - we simply need to know if the event fires. If it fires while a drag is occurring we can kill the drag

alexreardon · 2018-04-11T01:17:40Z

Thanks for your patience @grady-lad! Well done on your first contribution

grady-lad added 3 commits March 21, 2018 20:28

Cancel drag when window is not visible

303fdd8

Support for visibility change event for different browsers

703159b

Tests support for canceling drag when no visibility

490d7d1

alexreardon requested changes Mar 21, 2018

View reviewed changes

grady-lad added 3 commits March 27, 2018 10:42

Remove un-needed configuration for document visibility

6f6c861

Cleanup visibilty event implementation

3ed7605

Adding cancel visibility tests for each sensor

9760624

alexreardon and others added 2 commits April 10, 2018 16:15

Merge branch 'master' into cancel-drag-no-visibility

39beea0

refactoring cancel on visibility change

09e72e1

actually adding updated file

86a1fd8

alexreardon added 3 commits April 11, 2018 08:30

simplying page visibility check

bbbee0b

improving clarity

7d50f77

adding missing semi colin

c8a7ce8

alexreardon reviewed Apr 10, 2018

View reviewed changes

Merge branch 'master' into cancel-drag-no-visibility

6326881

alexreardon merged commit 5d56d2b into atlassian:master Apr 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel drag when no visibility #83 #396

Cancel drag when no visibility #83 #396

grady-lad commented Mar 21, 2018

alexreardon Mar 21, 2018

grady-lad Mar 23, 2018 •

edited

Loading

alexreardon Mar 21, 2018

alexreardon Mar 21, 2018

alexreardon Mar 21, 2018

grady-lad Mar 26, 2018

alexreardon Mar 21, 2018

alexreardon Mar 21, 2018

grady-lad Mar 26, 2018

alexreardon Mar 27, 2018

alexreardon commented Mar 21, 2018

alexreardon commented Mar 28, 2018

alexreardon commented Mar 28, 2018

grady-lad commented Mar 28, 2018

alexreardon commented Mar 29, 2018

alexreardon commented Apr 3, 2018

grady-lad commented Apr 4, 2018

alexreardon commented Apr 10, 2018

grady-lad commented Apr 10, 2018

alexreardon Apr 10, 2018

alexreardon Apr 10, 2018

alexreardon commented Apr 11, 2018

Cancel drag when no visibility #83 #396

Cancel drag when no visibility #83 #396

Conversation

grady-lad commented Mar 21, 2018

Quick Demo

Choose a reason for hiding this comment

grady-lad Mar 23, 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

alexreardon commented Mar 21, 2018

alexreardon commented Mar 28, 2018

alexreardon commented Mar 28, 2018

grady-lad commented Mar 28, 2018

alexreardon commented Mar 29, 2018

alexreardon commented Apr 3, 2018

grady-lad commented Apr 4, 2018

alexreardon commented Apr 10, 2018

grady-lad commented Apr 10, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexreardon commented Apr 11, 2018

grady-lad Mar 23, 2018 •

edited

Loading