Skip to content
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
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,10 @@
"keycode": "^2.2.0",
"prop-types": "^15.6.1",
"prop-types-extra": "^1.0.1",
"react-overlays": "^0.9.3",
"react-is": "^18.3.1",
"react-overlays": "^5.2.0",
"react-prop-types": "^0.4.0",
"react-transition-group": "^2.0.0",
"react-transition-group": "^4.4.0",
Comment thread
coderabbitai[bot] marked this conversation as resolved.
"uncontrollable": "^5.0.0",
"warning": "^3.0.0"
},
Expand Down
35 changes: 0 additions & 35 deletions patches/react-overlays+0.9.3.patch

This file was deleted.

52 changes: 52 additions & 0 deletions patches/react-overlays+5.2.1.patch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The current versions of react-overlays and react-transition-group both use findDOMNode, so I've had to update them. In some cases, they still might try to call it if we pass a React element as a prop instead of an HTMLElement, but my hope is that we only ever use them through react-bootstrap so that won't happen.

The changes here are just porting the previous patch over to the new version of react-overlays. Note that this patch doesn't actually affect the code used at runtime, and the web app is going to actually have its own patch for this library

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
diff --git a/node_modules/react-overlays/cjs/Modal.js b/node_modules/react-overlays/cjs/Modal.js
index 2247ad0..71c9f4a 100644
--- a/node_modules/react-overlays/cjs/Modal.js
+++ b/node_modules/react-overlays/cjs/Modal.js
@@ -215,7 +215,7 @@ var Modal = /*#__PURE__*/(0, _react.forwardRef)(function (_ref, ref) {
}
});
var handleDocumentKeyDown = (0, _useEventCallback["default"])(function (e) {
- if (keyboard && e.keyCode === 27 && modal.isTopModal()) {
+ if (keyboard && (e.keyCode === 27 || e.key === 'Escape') && modal.isTopModal()) {
onEscapeKeyDown == null ? void 0 : onEscapeKeyDown(e);

if (!e.defaultPrevented) {
diff --git a/node_modules/react-overlays/cjs/useRootClose.js b/node_modules/react-overlays/cjs/useRootClose.js
index 3fc40a7..cc16674 100644
--- a/node_modules/react-overlays/cjs/useRootClose.js
+++ b/node_modules/react-overlays/cjs/useRootClose.js
@@ -66,7 +66,7 @@ function useRootClose(ref, onRootClose, _temp) {
}
});
var handleKeyUp = (0, _useEventCallback["default"])(function (e) {
- if (e.keyCode === escapeKeyCode) {
+ if (e.keyCode === escapeKeyCode || e.key === 'Escape') {
onClose(e);
}
});
diff --git a/node_modules/react-overlays/esm/Modal.js b/node_modules/react-overlays/esm/Modal.js
index 42f79e4..a12ebac 100644
--- a/node_modules/react-overlays/esm/Modal.js
+++ b/node_modules/react-overlays/esm/Modal.js
@@ -193,7 +193,7 @@ var Modal = /*#__PURE__*/forwardRef(function (_ref, ref) {
}
});
var handleDocumentKeyDown = useEventCallback(function (e) {
- if (keyboard && e.keyCode === 27 && modal.isTopModal()) {
+ if (keyboard && (e.keyCode === 27 || e.key === 'Escape') && modal.isTopModal()) {
onEscapeKeyDown == null ? void 0 : onEscapeKeyDown(e);

if (!e.defaultPrevented) {
diff --git a/node_modules/react-overlays/esm/useRootClose.js b/node_modules/react-overlays/esm/useRootClose.js
index 8c7882c..30d0d56 100644
--- a/node_modules/react-overlays/esm/useRootClose.js
+++ b/node_modules/react-overlays/esm/useRootClose.js
@@ -53,7 +53,7 @@ function useRootClose(ref, onRootClose, _temp) {
}
});
var handleKeyUp = useEventCallback(function (e) {
- if (e.keyCode === escapeKeyCode) {
+ if (e.keyCode === escapeKeyCode || e.key === 'Escape') {
onClose(e);
}
});
12 changes: 9 additions & 3 deletions src/CarouselItem.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import classNames from 'classnames';
import React from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import transition from 'dom-helpers/transition';

const propTypes = {
Expand Down Expand Up @@ -29,6 +28,7 @@ class CarouselItem extends React.Component {
direction: null
};

this.containerRef = React.createRef();
this.isUnmounted = false;
}

Expand All @@ -43,7 +43,7 @@ class CarouselItem extends React.Component {
const prevActive = prevProps.active;

if (!active && prevActive) {
transition.end(ReactDOM.findDOMNode(this), this.handleAnimateOutEnd);
transition.end(this.containerRef.current, this.handleAnimateOutEnd);
}

if (active !== prevActive) {
Expand Down Expand Up @@ -99,7 +99,13 @@ class CarouselItem extends React.Component {
classes[this.state.direction] = true;
}

return <div {...props} className={classNames(className, classes)} />;
return (
<div
ref={this.containerRef}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Some of these are straightforward because there's already a DOM node that we can attach a ref to instead of using findDOMNode

{...props}
className={classNames(className, classes)}
/>
);
}
}

Expand Down
61 changes: 44 additions & 17 deletions src/Collapse.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ import Transition, {

import capitalize from './utils/capitalize';
import createChainedFunction from './utils/createChainedFunction';
import { getElementRef, makeMergedRef } from './utils/mergeRefs';
import withRef from './utils/withRef';

const MARGINS = {
height: ['marginTop', 'marginBottom'],
Expand Down Expand Up @@ -134,6 +136,12 @@ const defaultProps = {
};

class Collapse extends React.Component {
constructor(props) {
super(props);

this.childRef = React.createRef();
}

getDimension() {
return typeof this.props.dimension === 'function'
? this.props.dimension()
Expand All @@ -146,31 +154,34 @@ class Collapse extends React.Component {
}

/* -- Expanding -- */
handleEnter = elem => {
elem.style[this.getDimension()] = '0';
handleEnter = () => {
this.childRef.current.style[this.getDimension()] = '0';
};

handleEntering = elem => {
handleEntering = () => {
const dimension = this.getDimension();
elem.style[dimension] = this._getScrollDimensionValue(elem, dimension);
this.childRef.current.style[dimension] = this._getScrollDimensionValue(
this.childRef.current,
dimension
);
};

handleEntered = elem => {
elem.style[this.getDimension()] = null;
handleEntered = () => {
this.childRef.current.style[this.getDimension()] = null;
};

/* -- Collapsing -- */
handleExit = elem => {
handleExit = () => {
const dimension = this.getDimension();
elem.style[dimension] = `${this.props.getDimensionValue(
this.childRef.current.style[dimension] = `${this.props.getDimensionValue(
dimension,
elem
this.childRef.current
)}px`;
triggerBrowserReflow(elem);
triggerBrowserReflow(this.childRef.current);
};

handleExiting = elem => {
elem.style[this.getDimension()] = '0';
handleExiting = () => {
this.childRef.current.style[this.getDimension()] = '0';
};

render() {
Expand All @@ -188,19 +199,34 @@ class Collapse extends React.Component {
delete props.dimension;
delete props.getDimensionValue;

const handleEnter = createChainedFunction(this.handleEnter, onEnter);
const handleEnter = createChainedFunction(
this.handleEnter,
withRef(onEnter, this.childRef)
);
const handleEntering = createChainedFunction(
this.handleEntering,
onEntering
withRef(onEntering, this.childRef)
);
const handleEntered = createChainedFunction(this.handleEntered, onEntered);
const handleExit = createChainedFunction(this.handleExit, onExit);
const handleExiting = createChainedFunction(this.handleExiting, onExiting);
const handleEntered = createChainedFunction(
this.handleEntered,
withRef(onEntered, this.childRef)
);
const handleExit = createChainedFunction(
this.handleExit,
withRef(onExit, this.childRef)
);
const handleExiting = createChainedFunction(
this.handleExiting,
withRef(onExiting, this.childRef)
);

const ref = makeMergedRef([this.childRef, getElementRef(children)]);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is a case where we need to inject a ref and hope that it returns a DOM node. I think this may cause an error if Transition doesn't forward its ref to an HTML element, but I don't think we pas anything as Transition other than Fade and Collapse, both of which now forward refs


return (
<Transition
{...props}
aria-expanded={props.role ? props.in : null}
nodeRef={this.childRef}
onEnter={handleEnter}
onEntering={handleEntering}
onEntered={handleEntered}
Expand All @@ -210,6 +236,7 @@ class Collapse extends React.Component {
{(state, innerProps) =>
React.cloneElement(children, {
...innerProps,
ref,
className: classNames(
className,
children.props.className,
Expand Down
79 changes: 39 additions & 40 deletions src/Dropdown.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import activeElement from 'dom-helpers/activeElement';
import contains from 'dom-helpers/query/contains';
import React, { cloneElement } from 'react';
import PropTypes from 'prop-types';
import ReactDOM from 'react-dom';
import all from 'prop-types-extra/lib/all';
import elementType from 'prop-types-extra/lib/elementType';
import isRequiredForA11y from 'prop-types-extra/lib/isRequiredForA11y';
Expand All @@ -16,6 +15,7 @@ import { bsClass as setBsClass, prefix } from './utils/bootstrapUtils';
import createChainedFunction from './utils/createChainedFunction';
import { exclusiveRoles, requiredRoles } from './utils/PropTypes';
import ValidComponentChildren from './utils/ValidComponentChildren';
import { getElementRef, makeMergedRef } from './utils/mergeRefs';

const TOGGLE_ROLE = DropdownToggle.defaultProps.bsRole;
const MENU_ROLE = DropdownMenu.defaultProps.bsRole;
Expand Down Expand Up @@ -123,6 +123,7 @@ class Dropdown extends React.Component {
this.handleClose = this.handleClose.bind(this);

this._focusInDropdown = false;
this.containerRef = React.createRef();
this.lastOpenEventType = null;
}

Expand All @@ -133,7 +134,7 @@ class Dropdown extends React.Component {
UNSAFE_componentWillUpdate(nextProps) {
if (!nextProps.open && this.props.open) {
this._focusInDropdown = contains(
ReactDOM.findDOMNode(this.menu),
this.containerRef.current.querySelector('[role=menu]'),
activeElement(document)
);
}
Expand All @@ -158,7 +159,9 @@ class Dropdown extends React.Component {
}

focus() {
const toggle = ReactDOM.findDOMNode(this.toggle);
const toggle = this.containerRef.current.querySelector(

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

In this case, I didn't think we can pass a ref around for this, so I went with searching the DOM and hoping that the a11y for the toggle button is set up correctly. We use this component in 1 place in the web app (which has tests thankfully) and one or two plugins, so I'm hoping this doesn't break

'[role=button][aria-haspopup]'
);

if (toggle && toggle.focus) {
toggle.focus();
Expand Down Expand Up @@ -237,11 +240,12 @@ class Dropdown extends React.Component {
}

renderMenu(child, { id, onSelect, rootCloseEvent, ...props }) {
let ref = c => {
this.menu = c;
};

ref = createChainedFunction(child.ref, ref);
const ref = makeMergedRef([
el => {
this.menu = el;
},
getElementRef(child)
]);

return cloneElement(child, {
...props,
Expand All @@ -259,15 +263,8 @@ class Dropdown extends React.Component {
}

renderToggle(child, props) {
let ref = c => {
this.toggle = c;
};

ref = createChainedFunction(child.ref, ref);

return cloneElement(child, {
...props,
ref,
bsClass: prefix(props, 'toggle'),
onClick: createChainedFunction(child.props.onClick, this.handleClick),
onKeyDown: createChainedFunction(
Expand Down Expand Up @@ -311,31 +308,33 @@ class Dropdown extends React.Component {
// underlying component, to allow it to render size and style variants.

return (
<Component {...props} className={classNames(className, classes)}>
{ValidComponentChildren.map(children, child => {
switch (child.props.bsRole) {
case TOGGLE_ROLE:
return this.renderToggle(child, {
id,
disabled,
open,
role,
bsClass
});
case MENU_ROLE:
return this.renderMenu(child, {
id,
open,
pullRight,
bsClass,
onSelect,
rootCloseEvent
});
default:
return child;
}
})}
</Component>
<div ref={this.containerRef} style={{ display: 'contents' }}>
<Component {...props} className={classNames(className, classes)}>
{ValidComponentChildren.map(children, child => {
switch (child.props.bsRole) {
case TOGGLE_ROLE:
return this.renderToggle(child, {
id,
disabled,
open,
role,
bsClass
});
case MENU_ROLE:
return this.renderMenu(child, {
id,
open,
pullRight,
bsClass,
onSelect,
rootCloseEvent
});
default:
return child;
}
})}
</Component>
</div>
);
}
}
Expand Down
Loading