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

[table] fix(Draggable): defensive checks for invalid child target refs #6255

Merged
merged 1 commit into from
Jun 28, 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
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export interface EditableTextProps extends IntentProps, Props {
* 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>;
elementRef?: React.Ref<HTMLDivElement>;

/** Whether the component is currently being edited. */
isEditing?: boolean;
Expand Down
122 changes: 63 additions & 59 deletions packages/table/src/headers/editableName.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@
import classNames from "classnames";
import * as React from "react";

import { EditableText, IntentProps, Props } from "@blueprintjs/core";
import { DISPLAYNAME_PREFIX, EditableText, IntentProps, Props } from "@blueprintjs/core";

import * as Classes from "../common/classes";

export interface EditableNameProps extends IntentProps, Props {
export interface EditableNameProps extends IntentProps, Props, React.RefAttributes<HTMLDivElement> {
/**
* The name displayed in the text box. Be sure to update this value when
* rendering this component after a confirmed change.
Expand Down Expand Up @@ -65,60 +65,64 @@ export interface EditableNameState {
*
* @see https://blueprintjs.com/docs/#table/api.editablename
*/
export class EditableName extends React.PureComponent<EditableNameProps, EditableNameState> {
public constructor(props: EditableNameProps) {
super(props);
this.state = {
dirtyName: props.name,
isEditing: false,
savedName: props.name,
};
}

public componentDidUpdate(prevProps: EditableNameProps) {
const { name } = this.props;
if (name !== prevProps.name) {
this.setState({ savedName: name, dirtyName: name });
}
}

public render() {
const { className, intent, name } = this.props;
const { isEditing, dirtyName, savedName } = this.state;
return (
<EditableText
className={classNames(className, Classes.TABLE_EDITABLE_NAME)}
defaultValue={name}
intent={intent}
minWidth={0}
onCancel={this.handleCancel}
onChange={this.handleChange}
onConfirm={this.handleConfirm}
onEdit={this.handleEdit}
placeholder=""
selectAllOnFocus={true}
value={isEditing ? dirtyName : savedName}
/>
);
}

private handleEdit = () => {
this.setState({ isEditing: true, dirtyName: this.state.savedName });
};

private handleCancel = (value: string) => {
// don't strictly need to clear the dirtyName, but it's better hygiene
this.setState({ isEditing: false, dirtyName: undefined });
this.props.onCancel?.(value, this.props.index);
};

private handleChange = (value: string) => {
this.setState({ dirtyName: value });
this.props.onChange?.(value, this.props.index);
};

private handleConfirm = (value: string) => {
this.setState({ isEditing: false, savedName: value, dirtyName: undefined });
this.props.onConfirm?.(value, this.props.index);
};
}
export const EditableName: React.FC<EditableNameProps> = React.forwardRef((props, ref) => {
const [dirtyName, setDirtyName] = React.useState(props.name);
const [isEditing, setIsEditing] = React.useState(false);
const [savedName, setSavedName] = React.useState(props.name);

React.useEffect(() => {
setDirtyName(props.name);
setSavedName(props.name);
}, [props.name]);

const handleEdit = React.useCallback(() => {
setIsEditing(true);
setDirtyName(savedName);
}, [savedName]);

const handleCancel = React.useCallback(
(value: string) => {
// don't strictly need to clear the dirtyName, but it's better hygiene
setIsEditing(false);
setDirtyName(undefined);
props.onCancel?.(value, props.index);
},
[props.onCancel, props.index],
);

const handleChange = React.useCallback(
(value: string) => {
setDirtyName(value);
props.onChange?.(value, props.index);
},
[props.onChange, props.index],
);

const handleConfirm = React.useCallback(
(value: string) => {
setIsEditing(false);
setSavedName(value);
setDirtyName(undefined);
props.onConfirm?.(value, props.index);
},
[props.onConfirm, props.index],
);

return (
<EditableText
className={classNames(props.className, Classes.TABLE_EDITABLE_NAME)}
defaultValue={props.name}
elementRef={ref}
intent={props.intent}
minWidth={0}
onCancel={handleCancel}
onChange={handleChange}
onConfirm={handleConfirm}
onEdit={handleEdit}
placeholder=""
selectAllOnFocus={true}
value={isEditing ? dirtyName : savedName}
/>
);
});
EditableName.displayName = `${DISPLAYNAME_PREFIX}.EditableName`;
23 changes: 19 additions & 4 deletions packages/table/src/interactions/draggable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -74,19 +74,34 @@ export class Draggable extends React.PureComponent<DraggableProps> {
}

public componentDidUpdate(prevProps: DraggableProps) {
const propsWhitelist = { include: REATTACH_PROPS_KEYS };
if (!CoreUtils.shallowCompareKeys(prevProps, this.props, propsWhitelist) && this.targetRef.current != null) {
const didEventHandlerPropsChange = !CoreUtils.shallowCompareKeys(prevProps, this.props, {
include: REATTACH_PROPS_KEYS,
});
if (didEventHandlerPropsChange && isValidTarget(this.targetRef.current)) {
this.events.attach(this.targetRef.current, this.props);
}
}

public componentDidMount() {
if (this.targetRef.current != null) {
if (isValidTarget(this.targetRef.current)) {
this.events.attach(this.targetRef.current, this.props);
}
}

public componentWillUnmount() {
this.events?.detach();
if (isValidTarget(this.targetRef.current)) {
this.events?.detach();
}
}
}

/**
* Used to ensure that target elements are valid at runtime for use by DragEvents.
* This check is done at runtime instead of compile time because of our use of `React.cloneElement<any>()`
* and the associated untyped `ref` injection.
*
* @see https://github.com/palantir/blueprint/issues/6248
*/
function isValidTarget(refElement: React.RefObject<HTMLElement>["current"]): refElement is HTMLElement {
return refElement != null && refElement instanceof HTMLElement;
}