Fix issue #32252: Clicking in the body of a Markdown component does n…

…ot put it into edit mode
apache · Feb 26, 2025 · ce1e064 · ce1e064
1 parent dae6acf
commit ce1e064
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 18 deletions.
diff --git a/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx b/superset-frontend/src/dashboard/components/gridComponents/Markdown.jsx
@@ -123,6 +123,7 @@ class Markdown extends PureComponent {
       markdownSource: props.component.meta.code,
       editor: null,
       editorMode: 'preview',
+      isEditing: false,
       undoLength: props.undoLength,
       redoLength: props.redoLength,
     };
@@ -225,7 +226,9 @@ class Markdown extends PureComponent {
     const nextState = {
       ...this.state,
       editorMode: mode,
+      isEditing: mode === 'edit',
     };
+
     if (mode === 'preview') {
       this.updateMarkdownContent();
       nextState.hasError = false;
@@ -277,7 +280,10 @@ class Markdown extends PureComponent {
         width="100%"
         height="100%"
         showGutter={false}
-        editorProps={{ $blockScrolling: true }}
+        editorProps={{
+          $blockScrolling: true,
+          role: 'textbox',
+        }}
         value={
           // this allows "select all => delete" to give an empty editor
           typeof this.state.markdownSource === 'string'
@@ -308,7 +314,7 @@ class Markdown extends PureComponent {
   }
 
   render() {
-    const { isFocused, editorMode } = this.state;
+    const { isFocused, isEditing } = this.state;
 
     const {
       component,
@@ -329,8 +335,6 @@ class Markdown extends PureComponent {
         ? parentComponent.meta.width || GRID_MIN_COLUMN_COUNT
         : component.meta.width || GRID_MIN_COLUMN_COUNT;
 
-    const isEditing = editorMode === 'edit';
-
     return (
       <Draggable
         component={component}
@@ -382,6 +386,14 @@ class Markdown extends PureComponent {
                   ref={dragSourceRef}
                   className="dashboard-component dashboard-component-chart-holder"
                   data-test="dashboard-component-chart-holder"
+                  role="button"
+                  tabIndex="0"
+                  onClick={() => {
+                    if (editMode) {
+                      this.handleChangeFocus(true);
+                      this.handleChangeEditorMode('edit');
+                    }
+                  }}
                 >
                   {editMode && (
                     <HoverMenu position="top">
@@ -390,9 +402,7 @@ class Markdown extends PureComponent {
                       />
                     </HoverMenu>
                   )}
-                  {editMode && isEditing
-                    ? this.renderEditMode()
-                    : this.renderPreviewMode()}
+                  {isEditing ? this.renderEditMode() : this.renderPreviewMode()}
                 </div>
               </ResizableContainer>
             </MarkdownStyles>
@@ -414,4 +424,4 @@ function mapStateToProps(state) {
     htmlSchemaOverrides: state.common.conf.HTML_SANITIZATION_SCHEMA_EXTENSIONS,
   };
 }
-export default connect(mapStateToProps)(Markdown);
+export default connect(mapStateToProps)(Markdown);
diff --git a/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/WithPopoverMenu.tsx
@@ -83,7 +83,7 @@ const PopoverMenuStyles = styled.div`
       )};
     font-size: ${theme.typography.sizes.m}px;
     cursor: default;
-    z-index: 3000;
+    z-index: 100000;
 
     &,
     .menu-item {
@@ -156,15 +156,18 @@ export default class WithPopoverMenu extends PureComponent<
     if (!this.props.editMode) {
       return;
     }
-    const {
-      onChangeFocus,
-      shouldFocus: shouldFocusFunc,
-      disableClick,
-    } = this.props;
 
-    const shouldFocus = shouldFocusFunc(event, this.container);
+    // Allow the event to propagate to the Markdown component
+    const { shouldFocus } = this.props;
+    const shouldFocusComponent = shouldFocus(event, this.container);
 
-    if (!disableClick && shouldFocus && !this.state.isFocused) {
+    if (shouldFocusComponent) {
+      event.stopPropagation();
+    }
+
+    const { onChangeFocus, disableClick } = this.props;
+
+    if (!disableClick && shouldFocusComponent && !this.state.isFocused) {
       // if not focused, set focus and add a window event listener to capture outside clicks
       // this enables us to not set a click listener for ever item on a dashboard
       document.addEventListener('click', this.handleClick);
@@ -173,7 +176,7 @@ export default class WithPopoverMenu extends PureComponent<
       if (onChangeFocus) {
         onChangeFocus(true);
       }
-    } else if (!shouldFocus && this.state.isFocused) {
+    } else if (!shouldFocusComponent && this.state.isFocused) {
       document.removeEventListener('click', this.handleClick);
       document.removeEventListener('drag', this.handleClick);
       this.setState(() => ({ isFocused: false }));
@@ -211,4 +214,4 @@ export default class WithPopoverMenu extends PureComponent<
       </WithPopoverMenuStyles>
     );
   }
-}
+}