You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/08/27 10:30:45 UTC
(superset) branch master updated: fix(Modal): stop the draggable modal from hijacking text selection (#43498)
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 7d355a254ea fix(Modal): stop the draggable modal from hijacking text selection (#43498)
7d355a254ea is described below
commit 7d355a254ead7e9a9cd2cac1fa343d4831bde533
Author: Evan Rusackas <ev...@preset.io>
AuthorDate: Thu Aug 27 03:30:23 2026 -0700
fix(Modal): stop the draggable modal from hijacking text selection (#43498)
Co-authored-by: Claude Opus 4.8 <no...@anthropic.com>
---
.../src/components/Modal/Modal.test.tsx | 153 +++++++++++++++++++++
.../src/components/Modal/Modal.tsx | 25 ++--
2 files changed, 162 insertions(+), 16 deletions(-)
diff --git a/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.test.tsx b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.test.tsx
new file mode 100644
index 00000000000..e5faca64b9e
--- /dev/null
+++ b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.test.tsx
@@ -0,0 +1,153 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { useState } from 'react';
+import { fireEvent, render, screen } from '@superset-ui/core/spec';
+import { Input } from '../Input';
+import { Modal } from './Modal';
+
+const drag = (
+ target: Element,
+ from: [number, number],
+ to: [number, number],
+) => {
+ fireEvent.mouseDown(target, { clientX: from[0], clientY: from[1] });
+ fireEvent.mouseMove(document, { clientX: to[0], clientY: to[1] });
+ fireEvent.mouseUp(document);
+};
+
+const isDragged = () => !!document.querySelector('.react-draggable-dragged');
+
+describe('Modal draggable', () => {
+ test('dragging from the title bar moves the modal', () => {
+ render(
+ <Modal show onHide={() => {}} title="Edit Dataset" draggable name="test">
+ <Input data-test="field" defaultValue="value" />
+ </Modal>,
+ );
+
+ const trigger = document.querySelector('.draggable-trigger') as HTMLElement;
+ drag(trigger, [100, 50], [150, 90]);
+
+ expect(isDragged()).toBe(true);
+ });
+
+ test('dragging inside modal content does not move the modal', () => {
+ render(
+ <Modal show onHide={() => {}} title="Edit Dataset" draggable name="test">
+ <Input data-test="field" defaultValue="first_view_event" />
+ </Modal>,
+ );
+
+ const input = screen.getByTestId('field');
+ drag(input, [200, 400], [260, 430]);
+
+ expect(isDragged()).toBe(false);
+ });
+
+ test('dragging inside modal content does not move the modal, even after an unrelated re-render while the title was hovered', () => {
+ // Regression test: the title bar used to gate dragging with a
+ // hover-tracked boolean (mouseover/mouseout on `.draggable-trigger`)
+ // instead of react-draggable's own `handle` prop. Because the title
+ // element was defined as an inline component recreated on every
+ // render, any unrelated state change while the cursor was over the
+ // title (e.g. typing in any field) force-remounted it without a real
+ // mouseout ever firing, leaving dragging permanently enabled -- so
+ // selecting text anywhere in the modal dragged the whole modal
+ // instead.
+ function Harness() {
+ const [tick, setTick] = useState(0);
+ return (
+ <Modal
+ show
+ onHide={() => {}}
+ title="Edit Dataset"
+ draggable
+ name="test"
+ >
+ <button
+ type="button"
+ data-test="rerender"
+ onClick={() => setTick(tick + 1)}
+ >
+ rerender
+ </button>
+ <Input data-test="field" defaultValue="first_view_event" />
+ </Modal>
+ );
+ }
+
+ render(<Harness />);
+
+ const trigger = document.querySelector('.draggable-trigger') as HTMLElement;
+ fireEvent.mouseOver(trigger);
+ fireEvent.click(screen.getByTestId('rerender'));
+
+ const input = screen.getByTestId('field');
+ drag(input, [200, 400], [260, 430]);
+
+ expect(isDragged()).toBe(false);
+ });
+
+ test('dragging is disabled entirely when draggable is not set', () => {
+ render(
+ <Modal show onHide={() => {}} title="Edit Dataset" name="test">
+ <Input data-test="field" defaultValue="value" />
+ </Modal>,
+ );
+
+ expect(document.querySelector('.draggable-trigger')).toBeNull();
+ });
+
+ test('draggableConfig cannot re-enable dragging on a non-draggable modal', () => {
+ render(
+ <Modal
+ show
+ onHide={() => {}}
+ title="Edit Dataset"
+ name="test"
+ draggableConfig={{ disabled: false }}
+ >
+ <Input data-test="field" defaultValue="value" />
+ </Modal>,
+ );
+
+ expect(document.querySelector('.draggable-trigger')).toBeNull();
+ });
+
+ test('draggableConfig can still opt a draggable modal out of dragging', () => {
+ render(
+ <Modal
+ show
+ onHide={() => {}}
+ title="Edit Dataset"
+ draggable
+ name="test"
+ draggableConfig={{ disabled: true }}
+ >
+ <Input data-test="field" defaultValue="value" />
+ </Modal>,
+ );
+
+ const trigger = document.querySelector('.draggable-trigger') as HTMLElement;
+ drag(trigger, [100, 50], [150, 90]);
+
+ expect(isDragged()).toBe(false);
+ });
+});
diff --git a/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
index 34581940331..dc51efc1203 100644
--- a/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
+++ b/superset-frontend/packages/superset-ui-core/src/components/Modal/Modal.tsx
@@ -269,7 +269,6 @@ const CustomModal = ({
);
const draggableRef = useRef<HTMLDivElement>(null);
const [bounds, setBounds] = useState<DraggableBounds>({});
- const [dragDisabled, setDragDisabled] = useState<boolean>(true);
const theme = useTheme();
const handleOnHide = () => {
@@ -339,19 +338,7 @@ const CustomModal = ({
}, [hideFooter, resizableConfig]);
const ModalTitle = () =>
- draggable ? (
- <div
- className="draggable-trigger"
- onMouseOver={() => dragDisabled && setDragDisabled(false)}
- onMouseOut={() => !dragDisabled && setDragDisabled(true)}
- onFocus={() => dragDisabled && setDragDisabled(false)}
- onBlur={() => !dragDisabled && setDragDisabled(true)}
- >
- {title}
- </div>
- ) : (
- <>{title}</>
- );
+ draggable ? <div className="draggable-trigger">{title}</div> : <>{title}</>;
return (
<StyledModal
@@ -378,13 +365,19 @@ const CustomModal = ({
modalRender={modal =>
resizable || draggable ? (
<Draggable
- disabled={!draggable || dragDisabled}
bounds={bounds ?? false}
onStart={(event, uiData) => onDragStart(event, uiData)}
+ {...draggableConfig}
+ // `disabled` and `handle` are applied after the spread so callers
+ // can't use `draggableConfig` to re-enable dragging on a
+ // non-draggable modal or move the drag handle off the title bar.
+ // A caller opting a draggable modal out via
+ // `draggableConfig.disabled` is still honored.
+ disabled={!draggable || !!draggableConfig?.disabled}
+ handle={draggable ? '.draggable-trigger' : undefined}
// Pass nodeRef so react-draggable does not fall back to
// ReactDOM.findDOMNode (deprecated in React 18+ Strict Mode).
nodeRef={draggableRef}
- {...draggableConfig}
>
{resizable ? (
<Resizable className="resizable" {...getResizableConfig}>