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

Viewport dimensions #1068

Merged
merged 10 commits into from
Jan 25, 2019
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
22 changes: 11 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -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
}
}
}
Expand Down
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 3 additions & 7 deletions src/view/announcer/announcer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -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;
};
Expand Down
38 changes: 38 additions & 0 deletions src/view/drag-drop-context/check-doctype.js
Original file line number Diff line number Diff line change
@@ -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}
`);
}
};
2 changes: 2 additions & 0 deletions src/view/drag-drop-context/drag-drop-context.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 8 additions & 0 deletions src/view/get-body-element.js
Original file line number Diff line number Diff line change
@@ -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;
};
8 changes: 8 additions & 0 deletions src/view/get-document-element.js
Original file line number Diff line number Diff line change
@@ -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;
};
5 changes: 2 additions & 3 deletions src/view/window/get-max-window-scroll.js
Original file line number Diff line number Diff line change
@@ -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
Expand Down
8 changes: 4 additions & 4 deletions src/view/window/get-viewport.js
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
36 changes: 36 additions & 0 deletions test/unit/view/drag-drop-context/check-doctype.spec.js
Original file line number Diff line number Diff line change
@@ -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();
});
2 changes: 1 addition & 1 deletion test/unit/view/is-type-of-element/util/get-svg.js
Original file line number Diff line number Diff line change
@@ -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');
13 changes: 2 additions & 11 deletions test/utils/viewport.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down
6 changes: 3 additions & 3 deletions website/src/components/draggable-logo.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ type Props = {|
size: number,
|};

const getBody = (): HTMLBodyElement => {
const getBodyElement = (): HTMLBodyElement => {
invariant(document.body);
return document.body;
};
Expand All @@ -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;
}

Expand Down
6 changes: 3 additions & 3 deletions website/src/components/sidebar/reorderable-links.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import type {
DropResult,
} from '../../../../src';

const getBody = (): HTMLBodyElement => {
const getBodyElement = (): HTMLBodyElement => {
invariant(document.body);
return document.body;
};
Expand All @@ -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;
}

Expand Down