From a54576370cc8f20f20e282879f0cea945202876c Mon Sep 17 00:00:00 2001 From: Wengxin Xu Date: Tue, 25 Nov 2025 17:38:55 -0500 Subject: [PATCH 1/3] refactor(dashboard): migrate header component to TypeScript --- .../{Header.test.jsx => Header.test.tsx} | 55 +++++++-- .../Header/{Header.jsx => Header.tsx} | 116 +++++++++++------- .../Header/{index.js => index.ts} | 0 3 files changed, 116 insertions(+), 55 deletions(-) rename superset-frontend/src/dashboard/components/gridComponents/Header/{Header.test.jsx => Header.test.tsx} (74%) rename superset-frontend/src/dashboard/components/gridComponents/Header/{Header.jsx => Header.tsx} (71%) rename superset-frontend/src/dashboard/components/gridComponents/Header/{index.js => index.ts} (100%) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.jsx b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx similarity index 74% rename from superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.jsx rename to superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx index 5dea9221c733..ad8344b20bf3 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx @@ -7,7 +7,7 @@ * "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 + * 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 @@ -33,31 +33,61 @@ import Header from './Header'; // eslint-disable-next-line no-restricted-globals -- TODO: Migrate from describe blocks describe('Header', () => { - const props = { + interface HeaderTestProps { + id: string; + dashboardId: string; + parentId: string; + component: any; + depth: number; + parentComponent: any; + index: number; + editMode: boolean; + embeddedMode: boolean; + filters: Record; + handleComponentDrop: () => void; + deleteComponent: sinon.SinonSpy; + updateComponents: sinon.SinonSpy; + } + + const baseComponent = newComponentFactory(HEADER_TYPE); + const props: HeaderTestProps = { id: 'id', + dashboardId: '1', parentId: 'parentId', - component: newComponentFactory(HEADER_TYPE), + component: { + ...baseComponent, + id: 'id', + meta: { + ...(baseComponent.meta || {}), + text: 'New Title', + } + }, depth: 1, parentComponent: newComponentFactory(DASHBOARD_GRID_TYPE), index: 0, editMode: false, embeddedMode: false, filters: {}, - handleComponentDrop() {}, - deleteComponent() {}, - updateComponents() {}, + handleComponentDrop: () => {}, + deleteComponent: sinon.spy(), + updateComponents: sinon.spy(), }; - function setup(overrideProps) { + function setup(overrideProps: Partial = {}) { return render( -
+
, ); } + beforeEach(() => { + if (props.deleteComponent) props.deleteComponent.resetHistory(); + if (props.updateComponents) props.updateComponents.resetHistory(); + }); + test('should render a Draggable', () => { setup(); expect(screen.getByTestId('dragdroppable-object')).toBeInTheDocument(); @@ -97,9 +127,10 @@ describe('Header', () => { fireEvent.change(titleInput, { target: { value: 'New title' } }); fireEvent.blur(titleInput); - const headerId = props.component.id; + const headerId = props.id; expect(updateComponents.callCount).toBe(1); - expect(updateComponents.getCall(0).args[0][headerId].meta.text).toBe( + const componentUpdates = updateComponents.getCall(0).args[0] as Record; + expect(componentUpdates[headerId].meta.text).toBe( 'New title', ); }); @@ -114,8 +145,8 @@ describe('Header', () => { const deleteComponent = sinon.spy(); setup({ editMode: true, deleteComponent }); - const trashButton = screen.getByRole('img', { name: 'delete' }); - fireEvent.click(trashButton.parentElement); + const trashButton = screen.getByRole('button', { name: 'delete' }); + fireEvent.click(trashButton); expect(deleteComponent.callCount).toBe(1); }); diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.jsx b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx similarity index 71% rename from superset-frontend/src/dashboard/components/gridComponents/Header/Header.jsx rename to superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx index 37ddae5e3471..2e1f6d40212b 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.jsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx @@ -7,7 +7,7 @@ * "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 + * 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 @@ -17,7 +17,6 @@ * under the License. */ import { PureComponent } from 'react'; -import PropTypes from 'prop-types'; import cx from 'classnames'; import { css, styled } from '@apache-superset/core/ui'; @@ -32,30 +31,62 @@ import BackgroundStyleDropdown from 'src/dashboard/components/menu/BackgroundSty import DeleteComponentButton from 'src/dashboard/components/DeleteComponentButton'; import headerStyleOptions from 'src/dashboard/util/headerStyleOptions'; import backgroundStyleOptions from 'src/dashboard/util/backgroundStyleOptions'; -import { componentShape } from 'src/dashboard/util/propShapes'; import { SMALL_HEADER, BACKGROUND_TRANSPARENT, + MEDIUM_HEADER, + LARGE_HEADER, + BACKGROUND_WHITE, } from 'src/dashboard/util/constants'; +import * as componentTypes from 'src/dashboard/util/componentTypes'; -const propTypes = { - id: PropTypes.string.isRequired, - dashboardId: PropTypes.string.isRequired, - parentId: PropTypes.string.isRequired, - component: componentShape.isRequired, - depth: PropTypes.number.isRequired, - parentComponent: componentShape.isRequired, - index: PropTypes.number.isRequired, - editMode: PropTypes.bool.isRequired, - embeddedMode: PropTypes.bool.isRequired, +export type ComponentType = typeof componentTypes[keyof typeof componentTypes]; - // redux - handleComponentDrop: PropTypes.func.isRequired, - deleteComponent: PropTypes.func.isRequired, - updateComponents: PropTypes.func.isRequired, -}; +export type HeaderStyleValue = + | typeof SMALL_HEADER + | typeof MEDIUM_HEADER + | typeof LARGE_HEADER; -const defaultProps = {}; +export type BackgroundStyleValue = + | typeof BACKGROUND_TRANSPARENT + | typeof BACKGROUND_WHITE; + +export interface ComponentMeta { + width?: number; + height?: number; + text: string; + headerSize?: HeaderStyleValue; + background?: BackgroundStyleValue; + chartId?: number; + [key: string]: any; +} + +export interface ComponentShape { + id: string; + type: ComponentType; + parents?: string[]; + children?: string[]; + meta: ComponentMeta; +} + +interface HeaderProps { + id: string; + dashboardId: string; + parentId: string; + component: ComponentShape; + depth: number; + parentComponent: ComponentShape; + index: number; + editMode: boolean; + embeddedMode: boolean; + handleComponentDrop: (dropResult: any) => void; + deleteComponent: (id: string, parentId: string) => void; + updateComponents: (changes: Record) => void; +} + +interface HeaderState { + isFocused: boolean; +} const HeaderStyles = styled.div` ${({ theme }) => css` @@ -127,8 +158,12 @@ const HeaderStyles = styled.div` `} `; -class Header extends PureComponent { - constructor(props) { +class Header extends PureComponent { + handleChangeSize: (nextValue: string) => void; + handleChangeBackground: (nextValue: string) => void; + handleChangeText: (nextValue: string) => void; + + constructor(props: HeaderProps) { super(props); this.state = { isFocused: false, @@ -136,34 +171,32 @@ class Header extends PureComponent { this.handleDeleteComponent = this.handleDeleteComponent.bind(this); this.handleChangeFocus = this.handleChangeFocus.bind(this); this.handleUpdateMeta = this.handleUpdateMeta.bind(this); - this.handleChangeSize = this.handleUpdateMeta.bind(this, 'headerSize'); - this.handleChangeBackground = this.handleUpdateMeta.bind( - this, - 'background', - ); - this.handleChangeText = this.handleUpdateMeta.bind(this, 'text'); + + this.handleChangeSize = (nextValue: string) => this.handleUpdateMeta('headerSize', nextValue); + this.handleChangeBackground = (nextValue: string) => this.handleUpdateMeta('background', nextValue); + this.handleChangeText = (nextValue: string) => this.handleUpdateMeta('text', nextValue); } - handleChangeFocus(nextFocus) { + handleChangeFocus(nextFocus: boolean): void { this.setState(() => ({ isFocused: nextFocus })); } - handleUpdateMeta(metaKey, nextValue) { + handleUpdateMeta(metaKey: keyof ComponentMeta, nextValue: string): void { const { updateComponents, component } = this.props; - if (nextValue && component.meta[metaKey] !== nextValue) { - updateComponents({ + if (nextValue && component.meta[metaKey] !== nextValue) { + updateComponents({ [component.id]: { ...component, meta: { ...component.meta, - [metaKey]: nextValue, + [metaKey]: nextValue, }, }, - }); + } as Record); } } - handleDeleteComponent() { + handleDeleteComponent(): void { const { deleteComponent, id, parentId } = this.props; deleteComponent(id, parentId); } @@ -202,7 +235,7 @@ class Header extends PureComponent { disableDragDrop={isFocused} editMode={editMode} > - {({ dragSourceRef }) => ( + {({ dragSourceRef }: { dragSourceRef: React.Ref | undefined }) => (
{editMode && depth <= 2 && ( // drag handle looks bad when nested @@ -216,12 +249,12 @@ class Header extends PureComponent { , , ]} @@ -231,8 +264,8 @@ class Header extends PureComponent { className={cx( 'dashboard-component', 'dashboard-component-header', - headerStyle.className, - rowStyle.className, + headerStyle?.className, + rowStyle?.className, )} > {editMode && ( @@ -249,7 +282,7 @@ class Header extends PureComponent { showTooltip={false} /> {!editMode && !embeddedMode && ( - + )} @@ -260,7 +293,4 @@ class Header extends PureComponent { } } -Header.propTypes = propTypes; -Header.defaultProps = defaultProps; - export default Header; diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/index.js b/superset-frontend/src/dashboard/components/gridComponents/Header/index.ts similarity index 100% rename from superset-frontend/src/dashboard/components/gridComponents/Header/index.js rename to superset-frontend/src/dashboard/components/gridComponents/Header/index.ts From 87a7d02126fe455316a1c21e1e1e86af7e13345c Mon Sep 17 00:00:00 2001 From: Amy Li Date: Wed, 26 Nov 2025 01:05:10 -0500 Subject: [PATCH 2/3] style(dashboard): format header grid component --- .../gridComponents/Header/Header.test.tsx | 10 ++-- .../gridComponents/Header/Header.tsx | 57 +++++++++++-------- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx index ad8344b20bf3..bbbeca48b383 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx @@ -37,9 +37,9 @@ describe('Header', () => { id: string; dashboardId: string; parentId: string; - component: any; + component: any; depth: number; - parentComponent: any; + parentComponent: any; index: number; editMode: boolean; embeddedMode: boolean; @@ -50,7 +50,7 @@ describe('Header', () => { } const baseComponent = newComponentFactory(HEADER_TYPE); - const props: HeaderTestProps = { + const props: HeaderTestProps = { id: 'id', dashboardId: '1', parentId: 'parentId', @@ -69,8 +69,8 @@ describe('Header', () => { embeddedMode: false, filters: {}, handleComponentDrop: () => {}, - deleteComponent: sinon.spy(), - updateComponents: sinon.spy(), + deleteComponent: sinon.spy(), + updateComponents: sinon.spy(), }; function setup(overrideProps: Partial = {}) { diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx index 2e1f6d40212b..f61a2d1de143 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.tsx @@ -40,25 +40,26 @@ import { } from 'src/dashboard/util/constants'; import * as componentTypes from 'src/dashboard/util/componentTypes'; -export type ComponentType = typeof componentTypes[keyof typeof componentTypes]; +export type ComponentType = + (typeof componentTypes)[keyof typeof componentTypes]; -export type HeaderStyleValue = - | typeof SMALL_HEADER - | typeof MEDIUM_HEADER +export type HeaderStyleValue = + | typeof SMALL_HEADER + | typeof MEDIUM_HEADER | typeof LARGE_HEADER; -export type BackgroundStyleValue = - | typeof BACKGROUND_TRANSPARENT +export type BackgroundStyleValue = + | typeof BACKGROUND_TRANSPARENT | typeof BACKGROUND_WHITE; export interface ComponentMeta { width?: number; height?: number; - text: string; + text: string; headerSize?: HeaderStyleValue; background?: BackgroundStyleValue; chartId?: number; - [key: string]: any; + [key: string]: any; } export interface ComponentShape { @@ -162,7 +163,7 @@ class Header extends PureComponent { handleChangeSize: (nextValue: string) => void; handleChangeBackground: (nextValue: string) => void; handleChangeText: (nextValue: string) => void; - + constructor(props: HeaderProps) { super(props); this.state = { @@ -171,10 +172,13 @@ class Header extends PureComponent { this.handleDeleteComponent = this.handleDeleteComponent.bind(this); this.handleChangeFocus = this.handleChangeFocus.bind(this); this.handleUpdateMeta = this.handleUpdateMeta.bind(this); - - this.handleChangeSize = (nextValue: string) => this.handleUpdateMeta('headerSize', nextValue); - this.handleChangeBackground = (nextValue: string) => this.handleUpdateMeta('background', nextValue); - this.handleChangeText = (nextValue: string) => this.handleUpdateMeta('text', nextValue); + + this.handleChangeSize = (nextValue: string) => + this.handleUpdateMeta('headerSize', nextValue); + this.handleChangeBackground = (nextValue: string) => + this.handleUpdateMeta('background', nextValue); + this.handleChangeText = (nextValue: string) => + this.handleUpdateMeta('text', nextValue); } handleChangeFocus(nextFocus: boolean): void { @@ -183,16 +187,16 @@ class Header extends PureComponent { handleUpdateMeta(metaKey: keyof ComponentMeta, nextValue: string): void { const { updateComponents, component } = this.props; - if (nextValue && component.meta[metaKey] !== nextValue) { - updateComponents({ + if (nextValue && component.meta[metaKey] !== nextValue) { + updateComponents({ [component.id]: { ...component, meta: { ...component.meta, - [metaKey]: nextValue, + [metaKey]: nextValue, }, }, - } as Record); + } as Record); } } @@ -235,7 +239,11 @@ class Header extends PureComponent { disableDragDrop={isFocused} editMode={editMode} > - {({ dragSourceRef }: { dragSourceRef: React.Ref | undefined }) => ( + {({ + dragSourceRef, + }: { + dragSourceRef: React.Ref | undefined; + }) => (
{editMode && depth <= 2 && ( // drag handle looks bad when nested @@ -249,12 +257,12 @@ class Header extends PureComponent { , , ]} @@ -264,8 +272,8 @@ class Header extends PureComponent { className={cx( 'dashboard-component', 'dashboard-component-header', - headerStyle?.className, - rowStyle?.className, + headerStyle?.className, + rowStyle?.className, )} > {editMode && ( @@ -282,7 +290,10 @@ class Header extends PureComponent { showTooltip={false} /> {!editMode && !embeddedMode && ( - + )} From 9020833eb3635f40e29771082fdf097a83f30955 Mon Sep 17 00:00:00 2001 From: Amy Li Date: Fri, 28 Nov 2025 16:23:46 -0500 Subject: [PATCH 3/3] chore: apply prettier formatting fixes for frontend --- .../gridComponents/Header/Header.test.tsx | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx index bbbeca48b383..21aafeada076 100644 --- a/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/gridComponents/Header/Header.test.tsx @@ -60,7 +60,7 @@ describe('Header', () => { meta: { ...(baseComponent.meta || {}), text: 'New Title', - } + }, }, depth: 1, parentComponent: newComponentFactory(DASHBOARD_GRID_TYPE), @@ -77,15 +77,15 @@ describe('Header', () => { return render( -
+
, ); } beforeEach(() => { - if (props.deleteComponent) props.deleteComponent.resetHistory(); - if (props.updateComponents) props.updateComponents.resetHistory(); + if (props.deleteComponent) props.deleteComponent.resetHistory(); + if (props.updateComponents) props.updateComponents.resetHistory(); }); test('should render a Draggable', () => { @@ -129,10 +129,11 @@ describe('Header', () => { const headerId = props.id; expect(updateComponents.callCount).toBe(1); - const componentUpdates = updateComponents.getCall(0).args[0] as Record; - expect(componentUpdates[headerId].meta.text).toBe( - 'New title', - ); + const componentUpdates = updateComponents.getCall(0).args[0] as Record< + string, + any + >; + expect(componentUpdates[headerId].meta.text).toBe('New title'); }); test('should render a DeleteComponentButton when focused in editMode', () => {