From a5db99b01449fec63131bbc4b4230bfc424711b8 Mon Sep 17 00:00:00 2001 From: rtexelm Date: Wed, 20 Mar 2024 14:36:08 -0400 Subject: [PATCH 1/5] Add border to row on HoverMenu hover --- .../DashboardBuilder/DashboardBuilder.tsx | 5 +++++ .../dashboard/components/gridComponents/Row.jsx | 17 +++++++++++++++-- .../src/dashboard/components/menu/HoverMenu.tsx | 5 ++++- 3 files changed, 24 insertions(+), 3 deletions(-) diff --git a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx index e86d924b2104f..ba21420394b2e 100644 --- a/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx +++ b/superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx @@ -186,6 +186,11 @@ const DashboardContentWrapper = styled.div` pointer-events: none; } + .grid-row.grid-row--hovered:after, + .dashboard-component-tabs > .grid-row--hovered:after { + border: 2px dashed ${theme.colors.primary.base}; + } + .resizable-container { & .dashboard-component-chart-holder { .dashboard-chart { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index 95560e3b5900b..6ebfb164edc85 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -135,6 +135,7 @@ class Row extends React.PureComponent { this.state = { isFocused: false, isInView: false, + hoverMenuHovered: false, }; this.handleDeleteComponent = this.handleDeleteComponent.bind(this); this.handleUpdateMeta = this.handleUpdateMeta.bind(this); @@ -143,6 +144,7 @@ class Row extends React.PureComponent { 'background', ); this.handleChangeFocus = this.handleChangeFocus.bind(this); + this.handleMenuHover = this.handleMenuHover.bind(this); this.setVerticalEmptyContainerHeight = debounce( this.setVerticalEmptyContainerHeight.bind(this), FAST_DEBOUNCE, @@ -235,6 +237,12 @@ class Row extends React.PureComponent { deleteComponent(component.id, parentId); } + handleMenuHover() { + this.setState(prevState => ({ + hoverMenuHovered: !prevState.hoverMenuHovered, + })); + } + render() { const { component: rowComponent, @@ -252,7 +260,7 @@ class Row extends React.PureComponent { onChangeTab, isComponentVisible, } = this.props; - const { containerHeight } = this.state; + const { containerHeight, hoverMenuHovered } = this.state; const rowItems = rowComponent.children || []; @@ -287,7 +295,11 @@ class Row extends React.PureComponent { editMode={editMode} > {editMode && ( - + ; + onHover?: () => void; children: React.ReactNode; } @@ -71,7 +72,7 @@ export default class HoverMenu extends React.PureComponent { }; render() { - const { innerRef, position, children } = this.props; + const { innerRef, position, onHover, children } = this.props; return (
{ position === 'left' && 'hover-menu--left', position === 'top' && 'hover-menu--top', )} + onMouseEnter={onHover} + onMouseLeave={onHover} > {children}
From 1dc83cd419b02ae3d0d58242418df3bdd14bde6c Mon Sep 17 00:00:00 2001 From: rtexelm Date: Fri, 22 Mar 2024 19:54:38 -0400 Subject: [PATCH 2/5] Attempted to use a ref --- .../components/gridComponents/Row.jsx | 17 +++++++------ .../dashboard/components/menu/HoverMenu.tsx | 25 ++++++++++++++++--- 2 files changed, 30 insertions(+), 12 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index 6ebfb164edc85..3051138539003 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -144,13 +144,13 @@ class Row extends React.PureComponent { 'background', ); this.handleChangeFocus = this.handleChangeFocus.bind(this); - this.handleMenuHover = this.handleMenuHover.bind(this); this.setVerticalEmptyContainerHeight = debounce( this.setVerticalEmptyContainerHeight.bind(this), FAST_DEBOUNCE, ); this.containerRef = React.createRef(); + this.hoverRef = React.createRef(); this.observerEnabler = null; this.observerDisabler = null; } @@ -193,6 +193,13 @@ class Row extends React.PureComponent { componentDidUpdate() { this.setVerticalEmptyContainerHeight(); + console.log('old State', this.state.hoverMenuHovered); + console.log('old Ref', this.hoverRef.current?.hoveredState); + if (this.hoverRef.current) { + this.setState({ hoverMenuHovered: this.hoverRef.current.hovered }); + console.log('State', this.state.hoverMenuHovered); + console.log('Ref', this.hoverRef.current?.hovered); + } } setVerticalEmptyContainerHeight() { @@ -237,12 +244,6 @@ class Row extends React.PureComponent { deleteComponent(component.id, parentId); } - handleMenuHover() { - this.setState(prevState => ({ - hoverMenuHovered: !prevState.hoverMenuHovered, - })); - } - render() { const { component: rowComponent, @@ -297,8 +298,8 @@ class Row extends React.PureComponent { {editMode && ( diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx index 909f656d73a3e..dfd676989a43c 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx @@ -1,3 +1,4 @@ +/* eslint-disable react/no-unused-state */ /** * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file @@ -23,7 +24,6 @@ import cx from 'classnames'; interface HoverMenuProps { position: 'left' | 'top'; innerRef: RefObject; - onHover?: () => void; children: React.ReactNode; } @@ -71,8 +71,25 @@ export default class HoverMenu extends React.PureComponent { children: null, }; + constructor(props: HoverMenuProps) { + super(props); + // Disabling unused state rule as it is used in Row.jsx by hoverRef + // eslint-disable-next-line + this.state = { + hovered: false, + }; + } + + handleMouseEnter = () => { + this.setState({ hovered: true }); + }; + + handleMouseLeave = () => { + this.setState({ hovered: false }); + }; + render() { - const { innerRef, position, onHover, children } = this.props; + const { innerRef, position, children } = this.props; return (
{ position === 'left' && 'hover-menu--left', position === 'top' && 'hover-menu--top', )} - onMouseEnter={onHover} - onMouseLeave={onHover} + onMouseEnter={this.handleMouseEnter} + onMouseLeave={this.handleMouseLeave} > {children}
From fceca4386fd84d4ff09ab85bc29c4c8f628e3e57 Mon Sep 17 00:00:00 2001 From: rtexelm Date: Fri, 22 Mar 2024 20:30:45 -0400 Subject: [PATCH 3/5] Fix with callback dictated by HoverMenu --- .../components/gridComponents/Row.jsx | 15 ++++++--------- .../dashboard/components/menu/HoverMenu.tsx | 18 +++++++----------- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index 3051138539003..5a3a0ab93bcc4 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -144,13 +144,13 @@ class Row extends React.PureComponent { 'background', ); this.handleChangeFocus = this.handleChangeFocus.bind(this); + this.handleMenuHover = this.handleMenuHover.bind(this); this.setVerticalEmptyContainerHeight = debounce( this.setVerticalEmptyContainerHeight.bind(this), FAST_DEBOUNCE, ); this.containerRef = React.createRef(); - this.hoverRef = React.createRef(); this.observerEnabler = null; this.observerDisabler = null; } @@ -193,13 +193,6 @@ class Row extends React.PureComponent { componentDidUpdate() { this.setVerticalEmptyContainerHeight(); - console.log('old State', this.state.hoverMenuHovered); - console.log('old Ref', this.hoverRef.current?.hoveredState); - if (this.hoverRef.current) { - this.setState({ hoverMenuHovered: this.hoverRef.current.hovered }); - console.log('State', this.state.hoverMenuHovered); - console.log('Ref', this.hoverRef.current?.hovered); - } } setVerticalEmptyContainerHeight() { @@ -244,6 +237,10 @@ class Row extends React.PureComponent { deleteComponent(component.id, parentId); } + handleMenuHover = hovered => { + this.setState(() => ({ hoverMenuHovered: hovered })); + }; + render() { const { component: rowComponent, @@ -297,8 +294,8 @@ class Row extends React.PureComponent { > {editMode && ( diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx index dfd676989a43c..09cec71584eb5 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx @@ -25,6 +25,7 @@ interface HoverMenuProps { position: 'left' | 'top'; innerRef: RefObject; children: React.ReactNode; + onHover?: (isHovered: boolean) => void; } const HoverStyleOverrides = styled.div` @@ -71,21 +72,16 @@ export default class HoverMenu extends React.PureComponent { children: null, }; - constructor(props: HoverMenuProps) { - super(props); - // Disabling unused state rule as it is used in Row.jsx by hoverRef - // eslint-disable-next-line - this.state = { - hovered: false, - }; - } - handleMouseEnter = () => { - this.setState({ hovered: true }); + if (this.props.onHover) { + this.props.onHover(true); + } }; handleMouseLeave = () => { - this.setState({ hovered: false }); + if (this.props.onHover) { + this.props.onHover(false); + } }; render() { From 0d4bf69285681aaac626bf04e7bef31c3054df0e Mon Sep 17 00:00:00 2001 From: rtexelm Date: Wed, 27 Mar 2024 02:37:15 -0400 Subject: [PATCH 4/5] Add semantic-object-key-val as argument for onHover prop --- .../src/dashboard/components/gridComponents/Row.jsx | 3 ++- .../src/dashboard/components/menu/HoverMenu.tsx | 12 +++++++----- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx index 5a3a0ab93bcc4..b6dc042a2b094 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Row.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Row.jsx @@ -238,7 +238,8 @@ class Row extends React.PureComponent { } handleMenuHover = hovered => { - this.setState(() => ({ hoverMenuHovered: hovered })); + const { isHovered } = hovered; + this.setState(() => ({ hoverMenuHovered: isHovered })); }; render() { diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx index 09cec71584eb5..59c77bee282b6 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx @@ -25,7 +25,7 @@ interface HoverMenuProps { position: 'left' | 'top'; innerRef: RefObject; children: React.ReactNode; - onHover?: (isHovered: boolean) => void; + onHover?: (data: { isHovered: boolean }) => void; } const HoverStyleOverrides = styled.div` @@ -73,14 +73,16 @@ export default class HoverMenu extends React.PureComponent { }; handleMouseEnter = () => { - if (this.props.onHover) { - this.props.onHover(true); + const { onHover } = this.props; + if (onHover) { + onHover({ isHovered: true }); } }; handleMouseLeave = () => { - if (this.props.onHover) { - this.props.onHover(false); + const { onHover } = this.props; + if (onHover) { + onHover({ isHovered: false }); } }; From 60cc1c406a411fa8b4dc7f0520abb0b31dd05140 Mon Sep 17 00:00:00 2001 From: rtexelm Date: Wed, 27 Mar 2024 03:49:42 -0400 Subject: [PATCH 5/5] Add test for hover state --- .../dashboard/components/menu/HoverMenu.test.tsx | 16 +++++++++++++++- .../src/dashboard/components/menu/HoverMenu.tsx | 1 + 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.test.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.test.tsx index adf34938f55cd..5b7a64e64f8da 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.test.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.test.tsx @@ -17,7 +17,8 @@ * under the License. */ import React from 'react'; -import { render } from 'spec/helpers/testing-library'; +import { render, screen } from 'spec/helpers/testing-library'; +import userEvent from '@testing-library/user-event'; import HoverMenu from 'src/dashboard/components/menu/HoverMenu'; @@ -25,3 +26,16 @@ test('should render a div.hover-menu', () => { const { container } = render(); expect(container.querySelector('.hover-menu')).toBeInTheDocument(); }); + +test('should call onHover when mouse enters and leaves', () => { + const onHover = jest.fn(); + render(); + + const hoverMenu = screen.getByTestId('hover-menu'); + + userEvent.hover(hoverMenu); + expect(onHover).toBeCalledWith({ isHovered: true }); + + userEvent.unhover(hoverMenu); + expect(onHover).toBeCalledWith({ isHovered: false }); +}); diff --git a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx index 59c77bee282b6..43d7683ce370e 100644 --- a/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx +++ b/superset-frontend/src/dashboard/components/menu/HoverMenu.tsx @@ -99,6 +99,7 @@ export default class HoverMenu extends React.PureComponent { )} onMouseEnter={this.handleMouseEnter} onMouseLeave={this.handleMouseLeave} + data-test="hover-menu" > {children}