Viewport dimensions (#1068)

Fixes #1040
atlassian · Jan 25, 2019 · b0089d6 · b0089d6
1 parent baf7873
commit b0089d6
Show file tree

Hide file tree

Showing 16 changed files with 135 additions and 47 deletions.
diff --git a/.size-snapshot.json b/.size-snapshot.json
@@ -1,25 +1,25 @@
 {
   "dist/react-beautiful-dnd.js": {
-    "bundled": 346482,
-    "minified": 133786,
-    "gzipped": 39628
+    "bundled": 347275,
+    "minified": 134225,
+    "gzipped": 39798
   },
   "dist/react-beautiful-dnd.min.js": {
-    "bundled": 294956,
-    "minified": 109571,
-    "gzipped": 31915
+    "bundled": 295057,
+    "minified": 109593,
+    "gzipped": 31944
   },
   "dist/react-beautiful-dnd.esm.js": {
-    "bundled": 228188,
-    "minified": 120070,
-    "gzipped": 29868,
+    "bundled": 228983,
+    "minified": 120746,
+    "gzipped": 30051,
     "treeshaked": {
       "rollup": {
-        "code": 81899,
+        "code": 81922,
         "import_statements": 846
       },
       "webpack": {
-        "code": 84578
+        "code": 84599
       }
     }
   }

diff --git a/README.md b/README.md
@@ -1125,6 +1125,16 @@ resetServerContext();
 renderToString(...);
 ```
 
+## Use the html5 `doctype`
+
+Be sure that you have specified the html5 `doctype` (Document Type Definition - DTD) for your `html` page:
+
+```html
+<!DOCTYPE html>
+```
+
+A `doctype` impacts browser layout and measurement apis. Not specifying a `doctype` is a world of pain. Browsers will use some other `doctype` such as ["Quirks mode"](https://en.wikipedia.org/wiki/Quirks_mode) which can drastically change layout and measurement ([more information](https://www.w3.org/QA/Tips/Doctype)). The html5 `doctype` is our only supported `doctype`.
+
 ## Developer only warnings 👷‍
 
 For common setup and usage issues and errors `react-beautiful-dnd` will log some information `console` for development builds (`process.env.NODE_ENV !== 'production'`). These logs are stripped from productions builds to save kbs and to keep the `console` clean.

diff --git a/src/view/announcer/announcer.js b/src/view/announcer/announcer.js
@@ -3,6 +3,7 @@ import invariant from 'tiny-invariant';
 import type { Announce } from '../../types';
 import type { Announcer } from './announcer-types';
 import { warning } from '../../dev-warning';
+import getBodyElement from '../get-body-element';
 
 let count: number = 0;
 
@@ -21,11 +22,6 @@ const visuallyHidden: Object = {
   'clip-path': 'inset(100%)',
 };
 
-const getBody = (): HTMLBodyElement => {
-  invariant(document.body, 'Announcer cannot find document.body');
-  return document.body;
-};
-
 export default (): Announcer => {
   const id: string = `react-beautiful-dnd-announcement-${count++}`;
   let el: ?HTMLElement = null;
@@ -67,14 +63,14 @@ export default (): Announcer => {
     Object.assign(el.style, visuallyHidden);
 
     // Add to body
-    getBody().appendChild(el);
+    getBodyElement().appendChild(el);
   };
 
   const unmount = () => {
     invariant(el, 'Will not unmount announcer as it is already unmounted');
 
     // Remove from body
-    getBody().removeChild(el);
+    getBodyElement().removeChild(el);
     // Unset
     el = null;
   };

diff --git a/src/view/drag-drop-context/check-doctype.js b/src/view/drag-drop-context/check-doctype.js
@@ -0,0 +1,38 @@
+// @flow
+import { warning } from '../../dev-warning';
+
+const suffix: string = `
+  We expect a html5 doctype: <!doctype html>
+  This is to ensure consistent browser layout and measurement
+  More information:
+`;
+
+export default (doc: Document) => {
+  const doctype: ?DocumentType = doc.doctype;
+
+  if (!doctype) {
+    warning(`
+      No <!doctype html> found.
+
+      ${suffix}
+    `);
+    return;
+  }
+
+  if (doctype.name.toLowerCase() !== 'html') {
+    warning(`
+      Unexpected <!doctype> found: (${doctype.name})
+
+      ${suffix}
+    `);
+  }
+
+  if (doctype.publicId !== '') {
+    warning(`
+      Unexpected <!doctype> publicId found: (${doctype.publicId})
+      A html5 doctype does not have a publicId
+
+      ${suffix}
+    `);
+  }
+};
diff --git a/src/view/drag-drop-context/drag-drop-context.jsx b/src/view/drag-drop-context/drag-drop-context.jsx
@@ -39,6 +39,7 @@ import {
 import { getFormattedMessage } from '../../dev-warning';
 import { peerDependencies } from '../../../package.json';
 import checkReactVersion from './check-react-version';
+import checkDoctype from './check-doctype';
 
 type Props = {|
   ...Responders,
@@ -175,6 +176,7 @@ export default class DragDropContext extends React.Component<Props> {
 
     if (process.env.NODE_ENV !== 'production') {
       checkReactVersion(peerDependencies.react, React.version);
+      checkDoctype(document);
     }
   }
 

diff --git a/src/view/droppable-dimension-publisher/get-closest-scrollable.js b/src/view/droppable-dimension-publisher/get-closest-scrollable.js
@@ -1,6 +1,7 @@
 // @flow
 import invariant from 'tiny-invariant';
 import { warning } from '../../dev-warning';
+import getBodyElement from '../get-body-element';
 
 type Overflow = {|
   overflowX: string,
@@ -34,8 +35,7 @@ const isBodyScrollable = (): boolean => {
     return false;
   }
 
-  const body: ?HTMLBodyElement = document.body;
-  invariant(body);
+  const body: HTMLBodyElement = getBodyElement();
   const html: ?HTMLElement = document.documentElement;
   invariant(html);
 

diff --git a/src/view/get-body-element.js b/src/view/get-body-element.js
@@ -0,0 +1,8 @@
+// @flow
+import invariant from 'tiny-invariant';
+
+export default (): HTMLBodyElement => {
+  const body: ?HTMLBodyElement = document.body;
+  invariant(body, 'Cannot find document.body');
+  return body;
+};
diff --git a/src/view/get-document-element.js b/src/view/get-document-element.js
@@ -0,0 +1,8 @@
+// @flow
+import invariant from 'tiny-invariant';
+
+export default (): HTMLElement => {
+  const doc: ?HTMLElement = document.documentElement;
+  invariant(doc, 'Cannot find document.documentElement');
+  return doc;
+};
diff --git a/src/view/window/get-max-window-scroll.js b/src/view/window/get-max-window-scroll.js
@@ -1,11 +1,10 @@
 // @flow
 import type { Position } from 'css-box-model';
-import invariant from 'tiny-invariant';
 import getMaxScroll from '../../state/get-max-scroll';
+import getDocumentElement from '../get-document-element';
 
 export default (): Position => {
-  const doc: ?HTMLElement = document.documentElement;
-  invariant(doc, 'Cannot get max scroll without a document');
+  const doc: HTMLElement = getDocumentElement();
 
   const maxScroll: Position = getMaxScroll({
     // unclipped padding box, with scrollbar

diff --git a/src/view/window/get-viewport.js b/src/view/window/get-viewport.js
@@ -1,10 +1,10 @@
 // @flow
-import invariant from 'tiny-invariant';
 import { getRect, type Rect, type Position } from 'css-box-model';
 import type { Viewport } from '../../types';
 import { origin } from '../../state/position';
 import getWindowScroll from './get-window-scroll';
 import getMaxWindowScroll from './get-max-window-scroll';
+import getDocumentElement from '../get-document-element';
 
 export default (): Viewport => {
   const scroll: Position = getWindowScroll();
@@ -13,9 +13,9 @@ export default (): Viewport => {
   const top: number = scroll.y;
   const left: number = scroll.x;
 
-  const doc: ?HTMLElement = document.documentElement;
-  invariant(doc, 'Could not find document.documentElement');
-
+  // window.innerHeight: includes scrollbars (not what we want)
+  // document.clientHeight gives us the correct value when using the html5 doctype
+  const doc: HTMLElement = getDocumentElement();
   // Using these values as they do not consider scrollbars
   // padding box, without scrollbar
   const width: number = doc.clientWidth;

diff --git a/test/unit/state/middleware/update-viewport-max-scroll-on-destination-change.spec.js b/test/unit/state/middleware/update-viewport-max-scroll-on-destination-change.spec.js
@@ -23,11 +23,11 @@ import type {
 import getMaxScroll from '../../../../src/state/get-max-scroll';
 import { setViewport } from '../../../utils/viewport';
 import { initialPublishArgs, preset } from '../../../utils/preset-action-args';
+import getDocumentElement from '../../../../src/view/get-document-element';
 
 // using viewport from initial publish args
 const viewport: Viewport = initialPublishArgs.viewport;
-const doc: ?HTMLElement = document.documentElement;
-invariant(doc, 'Cannot find document');
+const doc: HTMLElement = getDocumentElement();
 
 const scrollHeight: number = viewport.frame.height;
 const scrollWidth: number = viewport.frame.width;

diff --git a/test/unit/view/drag-drop-context/check-doctype.spec.js b/test/unit/view/drag-drop-context/check-doctype.spec.js
@@ -0,0 +1,36 @@
+// @flow
+import { JSDOM } from 'jsdom';
+import checkDoctype from '../../../../src/view/drag-drop-context/check-doctype';
+
+jest.spyOn(console, 'warn').mockImplementation(() => {});
+
+afterEach(() => {
+  console.warn.mockClear();
+});
+
+it('should pass if using a html doctype', () => {
+  const jsdom = new JSDOM(`<!doctype html><p>Hello world</p>`);
+
+  checkDoctype(jsdom.window.document);
+
+  expect(console.warn).not.toHaveBeenCalled();
+});
+
+it('should fail if there is no doctype', () => {
+  const jsdom = new JSDOM(`<html><body>Hello world</body></html>`);
+
+  checkDoctype(jsdom.window.document);
+
+  expect(console.warn).toHaveBeenCalled();
+});
+
+it('should fail if there is a non-html5 doctype', () => {
+  // HTML 4.01 Strict
+  const jsdom = new JSDOM(
+    `<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd"><html><body>Hello world</body></html>`,
+  );
+
+  checkDoctype(jsdom.window.document);
+
+  expect(console.warn).toHaveBeenCalled();
+});
diff --git a/test/unit/view/is-type-of-element/util/get-svg.js b/test/unit/view/is-type-of-element/util/get-svg.js
@@ -1,4 +1,4 @@
 // @flow
 // $FlowFixMe - flow does not know what a SVGElement is
-export default (doc: HTMLDocument): SVGElement =>
+export default (doc: HTMLElement): SVGElement =>
   doc.createElementNS('http://www.w3.org/2000/svg', 'svg');
diff --git a/test/utils/viewport.js b/test/utils/viewport.js
@@ -3,16 +3,7 @@ import { type Rect, type Position } from 'css-box-model';
 import type { Viewport } from '../../src/types';
 import getViewport from '../../src/view/window/get-viewport';
 import getMaxScroll from '../../src/state/get-max-scroll';
-
-const getDoc = (): HTMLElement => {
-  const el: ?HTMLElement = document.documentElement;
-
-  if (!el) {
-    throw new Error('Unable to get document.documentElement');
-  }
-
-  return el;
-};
+import getDocumentElement from '../../src/view/get-document-element';
 
 export const setWindowScroll = (newScroll: Position) => {
   window.pageYOffset = newScroll.y;
@@ -29,7 +20,7 @@ export const setViewport = (viewport: Viewport) => {
 
   setWindowScroll(viewport.scroll.current);
 
-  const doc: HTMLElement = getDoc();
+  const doc: HTMLElement = getDocumentElement();
   doc.clientWidth = viewport.frame.width;
   doc.clientHeight = viewport.frame.height;
 

diff --git a/website/src/components/draggable-logo.jsx b/website/src/components/draggable-logo.jsx
@@ -104,7 +104,7 @@ type Props = {|
   size: number,
 |};
 
-const getBody = (): HTMLBodyElement => {
+const getBodyElement = (): HTMLBodyElement => {
   invariant(document.body);
   return document.body;
 };
@@ -121,12 +121,12 @@ class WithPortal extends React.Component<WithPortalProps> {
 
   componentDidMount() {
     const portal: HTMLElement = document.createElement('div');
-    getBody().appendChild(portal);
+    getBodyElement().appendChild(portal);
     this.portal = portal;
   }
 
   componentWillUnmount() {
-    getBody().removeChild(this.getPortal());
+    getBodyElement().removeChild(this.getPortal());
     this.portal = null;
   }
 

diff --git a/website/src/components/sidebar/reorderable-links.jsx b/website/src/components/sidebar/reorderable-links.jsx
@@ -14,7 +14,7 @@ import type {
   DropResult,
 } from '../../../../src';
 
-const getBody = (): HTMLBodyElement => {
+const getBodyElement = (): HTMLBodyElement => {
   invariant(document.body);
   return document.body;
 };
@@ -33,10 +33,10 @@ class PortalAwareLink extends React.Component<PortalAwareItemProps> {
   componentDidMount() {
     const portal: HTMLElement = document.createElement('div');
     this.portal = portal;
-    getBody().appendChild(portal);
+    getBodyElement().appendChild(portal);
   }
   componentWillUnmount() {
-    getBody().removeChild(this.getPortal());
+    getBodyElement().removeChild(this.getPortal());
     this.portal = null;
   }