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

[v5] [table] fix: stop using findDOMNode in Draggable interactions #6137

Merged
merged 6 commits into from
May 9, 2023
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
12 changes: 10 additions & 2 deletions packages/core/src/components/editable-text/editableText.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ export interface EditableTextProps extends IntentProps, Props {
*/
disabled?: boolean;

/**
* Ref to attach to the root element rendered by this component.
*
* N.B. this may be renamed to simply `ref` in a future major version of Blueprint, when this class component is
* refactored into a function.
*/
elementRef?: React.RefObject<HTMLDivElement>;

/** Whether the component is currently being edited. */
isEditing?: boolean;

Expand Down Expand Up @@ -205,7 +213,7 @@ export class EditableText extends AbstractPureComponent<EditableTextProps, Edita
}

public render() {
const { alwaysRenderInput, disabled, multiline, contentId } = this.props;
const { alwaysRenderInput, disabled, elementRef, multiline, contentId } = this.props;
const value = this.props.value ?? this.state.value;
const hasValue = value != null && value !== "";

Expand Down Expand Up @@ -247,7 +255,7 @@ export class EditableText extends AbstractPureComponent<EditableTextProps, Edita
const spanProps: React.HTMLProps<HTMLSpanElement> = contentId != null ? { id: contentId } : {};

return (
<div className={classes} onFocus={this.handleFocus} tabIndex={tabIndex}>
<div className={classes} onFocus={this.handleFocus} tabIndex={tabIndex} ref={elementRef}>
{alwaysRenderInput || this.state.isEditing ? this.renderInput(value) : undefined}
{shouldHideContents ? undefined : (
<span
Expand Down
40 changes: 18 additions & 22 deletions packages/core/src/components/popover/popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,7 @@ import classNames from "classnames";
import * as React from "react";
import { Manager, Modifier, Popper, PopperChildrenProps, Reference, ReferenceChildrenProps } from "react-popper";

import {
AbstractPureComponent,
Classes,
DISPLAYNAME_PREFIX,
HTMLDivProps,
mergeRefs,
refHandler,
Utils,
} from "../../common";
import { AbstractPureComponent, Classes, DISPLAYNAME_PREFIX, HTMLDivProps, refHandler, Utils } from "../../common";
import * as Errors from "../../common/errors";
import { Overlay } from "../overlay/overlay";
import { ResizeSensor } from "../resize-sensor/resizeSensor";
Expand Down Expand Up @@ -174,17 +166,20 @@ export class Popover<
* DOM element that contains the popover.
* When `usePortal={true}`, this element will be portaled outside the usual DOM flow,
* so this reference can be very useful for testing.
*
* @public for testing
*/
public popoverElement: HTMLElement | null = null;

/** DOM element that contains the target. */
public targetElement: HTMLElement | null = null;

/** Popover ref handler */
private popoverRef: React.Ref<HTMLDivElement> = refHandler(this, "popoverElement", this.props.popoverRef);

/** Target ref handler */
private targetRef: React.Ref<HTMLElement> = el => (this.targetElement = el);
/**
* Target DOM element ref.
*
* @public for testing
*/
public targetRef = React.createRef<HTMLElement>();

private cancelOpenTimeout?: () => void;

Expand Down Expand Up @@ -243,7 +238,7 @@ export class Popover<

return (
<Manager>
<Reference>{this.renderTarget}</Reference>
<Reference innerRef={this.targetRef}>{this.renderTarget}</Reference>
<Popper
innerRef={this.popoverRef}
placement={placement ?? positionToPlacement(position)}
Expand Down Expand Up @@ -319,7 +314,7 @@ export class Popover<
*/
public reposition = () => this.popperScheduleUpdate?.();

private renderTarget = ({ ref: popperChildRef }: ReferenceChildrenProps) => {
private renderTarget = ({ ref }: ReferenceChildrenProps) => {
const { children, className, fill, openOnTargetFocus, renderTarget } = this.props;
const { isOpen } = this.state;
const isControlled = this.isControlled();
Expand All @@ -330,8 +325,6 @@ export class Popover<
targetTagName = "div";
}

const ref = mergeRefs(popperChildRef, this.targetRef);
Copy link
Contributor Author

Choose a reason for hiding this comment

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


const targetEventHandlers: PopoverHoverTargetHandlers<T> | PopoverClickTargetHandlers<T> =
isHoverInteractionKind
? {
Expand Down Expand Up @@ -412,8 +405,10 @@ export class Popover<
target = wrappedTarget;
}

// N.B. we must attach the ref ('wrapped' with react-popper functionality) to the DOM element here and
// let ResizeSensor know about it
return (
<ResizeSensor targetRef={ref} onResize={this.reposition}>
<ResizeSensor targetRef={this.targetRef} onResize={this.reposition}>
{target}
</ResizeSensor>
);
Expand Down Expand Up @@ -670,14 +665,14 @@ export class Popover<
};

private handleOverlayClose = (e?: React.SyntheticEvent<HTMLElement>) => {
if (this.targetElement === null || e === undefined) {
if (this.targetRef.current == null || e === undefined) {
return;
}

const event = (e.nativeEvent ?? e) as Event;
const eventTarget = (event.composed ? event.composedPath()[0] : event.target) as HTMLElement;
// if click was in target, target event listener will handle things, so don't close
if (!Utils.elementIsOrContains(this.targetElement, eventTarget) || e.nativeEvent instanceof KeyboardEvent) {
if (!Utils.elementIsOrContains(this.targetRef.current, eventTarget) || e.nativeEvent instanceof KeyboardEvent) {
this.setOpenState(false, e);
}
};
Expand Down Expand Up @@ -716,7 +711,8 @@ export class Popover<

private updateDarkParent() {
if (this.props.usePortal && this.state.isOpen) {
const hasDarkParent = this.targetElement != null && this.targetElement.closest(`.${Classes.DARK}`) != null;
const hasDarkParent =
this.targetRef.current != null && this.targetRef.current.closest(`.${Classes.DARK}`) != null;
this.setState({ hasDarkParent });
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
@# Resize sensor

ResizeSensor observes the DOM and provides a callback for `"resize"` events on a single child element.
__ResizeSensor__ observes the DOM and provides a callback for `"resize"` events on a single child element.
It is a thin wrapper around [`ResizeObserver`][resizeobserver] to provide React bindings.

[resizeobserver]: https://developers.google.com/web/updates/2016/10/resizeobserver
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/components/resize-sensor/resizeSensor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export interface ResizeSensorProps {
* If you attach a `ref` to the child yourself when rendering it, you must pass the
* same value here (otherwise, ResizeSensor won't be able to attach its own).
*/
targetRef?: React.Ref<any>;
targetRef?: React.RefObject<HTMLElement>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

slight breaking change - see PR description for justification

}

/**
Expand All @@ -68,7 +68,7 @@ export interface ResizeSensorProps {
export class ResizeSensor extends AbstractPureComponent<ResizeSensorProps> {
public static displayName = `${DISPLAYNAME_PREFIX}.ResizeSensor`;

private targetRef = React.createRef<HTMLElement>();
private targetRef = this.props.targetRef ?? React.createRef<HTMLElement>();

private prevElement: HTMLElement | undefined = undefined;

Expand Down
2 changes: 1 addition & 1 deletion packages/core/test/popover/popoverTests.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ describe("<Popover>", () => {

const instance = wrapper.instance() as Popover<React.HTMLProps<HTMLButtonElement>>;
wrapper.popoverElement = instance.popoverElement!;
wrapper.targetElement = instance.targetElement!;
wrapper.targetElement = instance.targetRef.current!;
wrapper.assertFindClass = (className: string, expected = true, msg = className) => {
const actual = wrapper!.findClass(className);
if (expected) {
Expand Down
22 changes: 12 additions & 10 deletions packages/table/src/cell/editableCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export interface EditableCellProps extends CellProps {
/**
* Props that should be passed to the EditableText when it is used to edit
*/
editableTextProps?: EditableTextProps;
editableTextProps?: Omit<EditableTextProps, "elementRef">;
}

export interface EditableCellState {
Expand All @@ -90,13 +90,9 @@ export class EditableCell extends React.Component<EditableCellProps, EditableCel
wrapText: false,
};

private cellRef: HTMLElement | null | undefined;
private cellRef = React.createRef<HTMLDivElement>();

private refHandlers = {
cell: (ref: HTMLElement | null) => {
this.cellRef = ref;
},
};
private contentsRef = React.createRef<HTMLDivElement>();

public constructor(props: EditableCellProps) {
super(props);
Expand Down Expand Up @@ -146,6 +142,7 @@ export class EditableCell extends React.Component<EditableCellProps, EditableCel
{...editableTextProps}
isEditing={true}
className={classNames(Classes.TABLE_EDITABLE_TEXT, Classes.TABLE_EDITABLE_NAME, className)}
elementRef={this.contentsRef}
intent={spreadableProps.intent}
minWidth={0}
onCancel={this.handleCancel}
Expand All @@ -163,7 +160,11 @@ export class EditableCell extends React.Component<EditableCellProps, EditableCel
[Classes.TABLE_NO_WRAP_TEXT]: !wrapText,
});

cellContents = <div className={textClasses}>{savedValue}</div>;
cellContents = (
<div className={textClasses} ref={this.contentsRef}>
{savedValue}
</div>
);
}

return (
Expand All @@ -172,14 +173,15 @@ export class EditableCell extends React.Component<EditableCellProps, EditableCel
wrapText={wrapText}
truncated={false}
interactive={interactive}
cellRef={this.refHandlers.cell}
cellRef={this.cellRef}
onKeyPress={this.handleKeyPress}
>
<Draggable
onActivate={this.handleCellActivate}
onDoubleClick={this.handleCellDoubleClick}
preventDefault={false}
stopPropagation={interactive}
targetRef={this.contentsRef}
>
{cellContents}
</Draggable>
Expand All @@ -206,7 +208,7 @@ export class EditableCell extends React.Component<EditableCellProps, EditableCel
private checkShouldFocus() {
if (this.props.isFocused && !this.state.isEditing) {
// don't focus if we're editing -- we'll lose the fact that we're editing
this.cellRef?.focus();
this.cellRef.current?.focus();
}
}

Expand Down
22 changes: 12 additions & 10 deletions packages/table/src/cell/editableCell2.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ export interface EditableCell2Props extends Omit<CellProps, "onKeyDown" | "onKey
/**
* Props that should be passed to the EditableText when it is used to edit
*/
editableTextProps?: EditableTextProps;
editableTextProps?: Omit<EditableTextProps, "elementRef">;
}

export interface EditableCell2State {
Expand All @@ -92,13 +92,9 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
wrapText: false,
};

private cellRef: HTMLElement | null | undefined;
private cellRef = React.createRef<HTMLDivElement>();

private refHandlers = {
cell: (ref: HTMLElement | null) => {
this.cellRef = ref;
},
};
private contentsRef = React.createRef<HTMLDivElement>();

public state: EditableCell2State = {
isEditing: false,
Expand Down Expand Up @@ -157,6 +153,7 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
{...editableTextProps}
isEditing={true}
className={classNames(Classes.TABLE_EDITABLE_TEXT, Classes.TABLE_EDITABLE_NAME, className)}
elementRef={this.contentsRef}
intent={spreadableProps.intent}
minWidth={0}
onCancel={this.handleCancel}
Expand All @@ -174,7 +171,11 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
[Classes.TABLE_NO_WRAP_TEXT]: !wrapText,
});

cellContents = <div className={textClasses}>{savedValue}</div>;
cellContents = (
<div className={textClasses} ref={this.contentsRef}>
{savedValue}
</div>
);
}

return (
Expand All @@ -183,7 +184,7 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
wrapText={wrapText}
truncated={false}
interactive={interactive}
cellRef={this.refHandlers.cell}
cellRef={this.cellRef}
onKeyDown={handleKeyDown}
onKeyPress={this.handleKeyPress}
onKeyUp={handleKeyUp}
Expand All @@ -194,6 +195,7 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
onDoubleClick={this.handleCellDoubleClick}
preventDefault={false}
stopPropagation={interactive}
targetRef={this.contentsRef}
>
{cellContents}
</Draggable>
Expand All @@ -204,7 +206,7 @@ export class EditableCell2 extends React.Component<EditableCell2Props, EditableC
private checkShouldFocus() {
if (this.props.isFocused && !this.state.isEditing) {
// don't focus if we're editing -- we'll lose the fact that we're editing
this.cellRef?.focus();
this.cellRef.current?.focus();
}
}

Expand Down
5 changes: 3 additions & 2 deletions packages/table/src/common/contextMenuTargetWrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface ContextMenuTargetWrapperProps extends Props {
children?: React.ReactNode;
renderContextMenu: (e: React.MouseEvent<HTMLElement>) => JSX.Element | undefined;
style: React.CSSProperties;
targetRef?: React.RefObject<HTMLDivElement>;
}

/**
Expand All @@ -40,9 +41,9 @@ export interface ContextMenuTargetWrapperProps extends Props {
@ContextMenuTargetLegacy
export class ContextMenuTargetWrapper extends React.PureComponent<ContextMenuTargetWrapperProps> {
public render() {
const { className, children, style } = this.props;
const { className, children, targetRef, style } = this.props;
return (
<div className={className} style={style}>
<div className={className} style={style} ref={targetRef}>
{children}
</div>
);
Expand Down
4 changes: 2 additions & 2 deletions packages/table/src/deprecatedAliases.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ export {
/** @deprecated import ColumnHeaderCell instead */
ColumnHeaderCell as ColumnHeaderCell2,
/** @deprecated import ColumnHeaderCellProps instead */
ColumnHeaderCellProps as ColumnHeaderCellProps2,
type ColumnHeaderCellProps as ColumnHeaderCellProps2,
} from "./headers/columnHeaderCell";

export {
/** @deprecated import RowHeaderCell instead */
RowHeaderCell as RowHeaderCell2,
/** @deprecated import RowHeaderCellProps instead */
RowHeaderCellProps as RowHeaderCellProps2,
type RowHeaderCellProps as RowHeaderCellProps2,
} from "./headers/rowHeaderCell";
Loading