Skip to content
This repository has been archived by the owner on Dec 5, 2024. It is now read-only.

Commit

Permalink
Allow ref objects to be passed (#308)
Browse files Browse the repository at this point in the history
* Allow ref objects to be passed

* fix tests

* Added tests
  • Loading branch information
maclockard authored and FezVrasta committed Nov 2, 2019
1 parent ac37373 commit 9201c9b
Show file tree
Hide file tree
Showing 9 changed files with 256 additions and 60 deletions.
22 changes: 11 additions & 11 deletions .size-snapshot.json
Original file line number Diff line number Diff line change
@@ -1,25 +1,25 @@
{
"dist/index.umd.js": {
"bundled": 66802,
"minified": 23724,
"gzipped": 7169
"bundled": 67132,
"minified": 23802,
"gzipped": 7191
},
"dist/index.umd.min.js": {
"bundled": 31438,
"minified": 12846,
"gzipped": 4182
"bundled": 31768,
"minified": 12924,
"gzipped": 4205
},
"dist/index.esm.js": {
"bundled": 12271,
"minified": 7288,
"gzipped": 2066,
"bundled": 12577,
"minified": 7364,
"gzipped": 2096,
"treeshaked": {
"rollup": {
"code": 3754,
"code": 3832,
"import_statements": 137
},
"webpack": {
"code": 4860
"code": 4938
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,9 @@
"babel-jest": "^24.0.0",
"cross-env": "^5.1.4",
"emotion": "^9.1.1",
"enzyme": "^3.3.0",
"enzyme-adapter-react-16": "^1.1.1",
"enzyme-to-json": "^3.3.3",
"enzyme": "^3.10.0",
"enzyme-adapter-react-16": "^1.14.0",
"enzyme-to-json": "^3.4.0",
"eslint": "^4.19.1",
"eslint-config-prettier": "^2.9.0",
"eslint-plugin-flowtype": "^2.46.1",
Expand Down
14 changes: 7 additions & 7 deletions src/Popper.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@ import PopperJS, {
} from 'popper.js';
import type { Style } from 'typed-styles';
import { ManagerContext } from './Manager';
import { safeInvoke, unwrapArray, shallowEqual } from './utils';
import { unwrapArray, setRef, shallowEqual } from './utils';
import { type Ref } from "./RefTypes";

type getRefFn = (?HTMLElement) => void;
type ReferenceElement = ReferenceObject | HTMLElement | null;
type StyleOffsets = { top: number, left: number };
type StylePosition = { position: 'absolute' | 'fixed' };

export type PopperArrowProps = {
ref: getRefFn,
ref: Ref,
style: StyleOffsets & Style,
};
export type PopperChildrenProps = {|
ref: getRefFn,
ref: Ref,
style: StyleOffsets & StylePosition & Style,
placement: Placement,
outOfBoundaries: ?boolean,
Expand All @@ -33,7 +33,7 @@ export type PopperChildren = PopperChildrenProps => React.Node;
export type PopperProps = {
children: PopperChildren,
eventsEnabled?: boolean,
innerRef?: getRefFn,
innerRef?: Ref,
modifiers?: Modifiers,
placement?: Placement,
positionFixed?: boolean,
Expand Down Expand Up @@ -76,7 +76,7 @@ export class InnerPopper extends React.Component<PopperProps, PopperState> {
setPopperNode = (popperNode: ?HTMLElement) => {
if (!popperNode || this.popperNode === popperNode) return;

safeInvoke(this.props.innerRef, popperNode);
setRef(this.props.innerRef, popperNode);
this.popperNode = popperNode;

this.updatePopperInstance();
Expand Down Expand Up @@ -200,7 +200,7 @@ export class InnerPopper extends React.Component<PopperProps, PopperState> {
}

componentWillUnmount() {
safeInvoke(this.props.innerRef, null);
setRef(this.props.innerRef, null)
this.destroyPopperInstance();
}

Expand Down
34 changes: 34 additions & 0 deletions src/Popper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,40 @@ describe('Popper component', () => {
).not.toThrow();
});

it('accepts a ref function', () => {
const myRef = jest.fn();
const referenceElement = document.createElement('div');
mount(
<InnerPopper referenceElement={referenceElement} innerRef={myRef}>
{({ ref, style, placement}) => (
<div
ref={ref}
style={style}
data-placement={placement}
/>
)}
</InnerPopper>
)
expect(myRef).toBeCalled();
});

it('accepts a ref object', () => {
const myRef = (React: any).createRef();
const referenceElement = document.createElement('div');
mount(
<InnerPopper referenceElement={referenceElement} innerRef={myRef}>
{({ ref, style, placement}) => (
<div
ref={ref}
style={style}
data-placement={placement}
/>
)}
</InnerPopper>
)
expect(myRef.current).toBeDefined();
});

it('accepts a `referenceElement` property', () => {
class VirtualReference {
getBoundingClientRect() {
Expand Down
4 changes: 4 additions & 0 deletions src/RefTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
type RefHandler = (?HTMLElement) => void;
type RefObject = { current?: HTMLElement};

export type Ref = RefHandler | RefObject;
11 changes: 6 additions & 5 deletions src/Reference.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@
import * as React from 'react';
import warning from 'warning';
import { ManagerContext } from './Manager';
import { safeInvoke, unwrapArray } from './utils';
import { safeInvoke, unwrapArray, setRef } from './utils';
import { type Ref } from "./RefTypes";

export type ReferenceChildrenProps = { ref: (?HTMLElement) => void };
export type ReferenceChildrenProps = { ref: Ref };
export type ReferenceProps = {
children: ReferenceChildrenProps => React.Node,
innerRef?: (?HTMLElement) => void,
innerRef?: Ref,
};

type InnerReferenceProps = {
Expand All @@ -18,12 +19,12 @@ class InnerReference extends React.Component<
ReferenceProps & InnerReferenceProps
> {
refHandler = (node: ?HTMLElement) => {
safeInvoke(this.props.innerRef, node);
setRef(this.props.innerRef, node)
safeInvoke(this.props.setReferenceNode, node);
};

componentWillUnmount() {
safeInvoke(this.props.innerRef, null);
setRef(this.props.innerRef, null)
}

render() {
Expand Down
16 changes: 16 additions & 0 deletions src/utils.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
// @flow

import { type Ref } from "./RefTypes";

/**
* Takes an argument and if it's an array, returns the first item in the array,
* otherwise returns the argument. Used for Preact compatibility.
Expand Down Expand Up @@ -38,3 +40,17 @@ export const shallowEqual = (objA: { [key: string]: any}, objB: { [key: string]:

return true;
}

/**
* Sets a ref using either a ref callback or a ref object
*/
export const setRef = (ref: ?Ref, node: ?HTMLElement) => {
// if its a function call it
if (typeof ref === "function") {
return safeInvoke(ref, node);
}
// otherwise we should treat it as a ref object
else if (ref != null) {
ref.current = node;
}
}
15 changes: 8 additions & 7 deletions typings/react-popper.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,36 +6,37 @@ interface ManagerProps {
}
export class Manager extends React.Component<ManagerProps, {}> { }

type RefHandler = (ref: HTMLElement | SVGElement | null) => void;

interface ReferenceChildrenProps {
ref: RefHandler;
// React refs are supposed to be contravariant (allows a more general type to be passed rather than a more specific one)
// However, Typescript currently can't infer that fact for refs
// See https://github.com/microsoft/TypeScript/issues/30748 for more information
ref: React.Ref<any>;
}

interface ReferenceProps {
children: (props: ReferenceChildrenProps) => React.ReactNode;
innerRef?: RefHandler;
innerRef?: React.Ref<any>;
}
export class Reference extends React.Component<ReferenceProps, {}> { }

export interface PopperArrowProps {
ref: RefHandler;
ref: React.Ref<any>;
style: React.CSSProperties;
}

export interface PopperChildrenProps {
arrowProps: PopperArrowProps;
outOfBoundaries: boolean | null;
placement: PopperJS.Placement;
ref: RefHandler;
ref: React.Ref<any>;
scheduleUpdate: () => void;
style: React.CSSProperties;
}

export interface PopperProps {
children: (props: PopperChildrenProps) => React.ReactNode;
eventsEnabled?: boolean;
innerRef?: RefHandler;
innerRef?: React.Ref<any>;
modifiers?: PopperJS.Modifiers;
placement?: PopperJS.Placement;
positionFixed?: boolean;
Expand Down
Loading

2 comments on commit 9201c9b

@drewdecarme
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As an FYI, removing RefHandler is a breaking change that you released in a patch version. Those of us that are using TS and had imported RefHandler had our builds break.

@FezVrasta
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think it’s reasonable to expect semver on type annotations, TS changes so often introducing breaking changes in patch versions that it would be impossible to guarantee stability.

Please sign in to comment.