From 5bfc95e79e89961967ba4acc8d24131157ccd16b Mon Sep 17 00:00:00 2001 From: Diego Medina Date: Wed, 8 Jun 2022 18:52:53 -0400 Subject: [PATCH] feat: When editing the label/title in the Metrics popover, hitting Enter should save what you've typed (#19898) * feature: When editing the label/title in the Metrics popover, hitting Enter should save what you've typed * Apply emotion templating to input/input labels --- .../src/assets/stylesheets/superset.less | 6 - .../DndColumnSelectPopoverTitle.jsx | 11 +- .../AdhocMetricEditPopoverTitle.jsx | 115 -------------- .../AdhocMetricEditPopoverTitle.test.jsx | 70 --------- .../AdhocMetricEditPopoverTitle.test.tsx | 141 ++++++++++++++++++ .../AdhocMetricEditPopoverTitle.tsx | 127 ++++++++++++++++ 6 files changed, 276 insertions(+), 194 deletions(-) delete mode 100644 superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.jsx delete mode 100644 superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.jsx create mode 100644 superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.tsx create mode 100644 superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx diff --git a/superset-frontend/src/assets/stylesheets/superset.less b/superset-frontend/src/assets/stylesheets/superset.less index 0cf419b30d19..5808d0144bc7 100644 --- a/superset-frontend/src/assets/stylesheets/superset.less +++ b/superset-frontend/src/assets/stylesheets/superset.less @@ -518,12 +518,6 @@ tr.reactable-column-header th.reactable-header-sortable { padding-right: 17px; } -.metric-edit-popover-label-input { - border-radius: @border-radius-large; - height: 30px; - padding-left: 10px; -} - .align-right { text-align: right; } diff --git a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelectPopoverTitle.jsx b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelectPopoverTitle.jsx index eecce0b33335..b50abb9aae6e 100644 --- a/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelectPopoverTitle.jsx +++ b/superset-frontend/src/explore/components/controls/DndColumnSelectControl/DndColumnSelectPopoverTitle.jsx @@ -17,10 +17,16 @@ * under the License. */ import React, { useCallback, useState } from 'react'; -import { t } from '@superset-ui/core'; +import { t, styled } from '@superset-ui/core'; import { Input } from 'src/components/Input'; import { Tooltip } from 'src/components/Tooltip'; +const StyledInput = styled(Input)` + border-radius: ${({ theme }) => theme.borderRadius}; + height: 26px; + padding-left: ${({ theme }) => theme.gridUnit * 2.5}px; +`; + export const DndColumnSelectPopoverTitle = ({ title, onChange, @@ -63,8 +69,7 @@ export const DndColumnSelectPopoverTitle = ({ } return isEditMode ? ( - {title.label || defaultLabel} - ); - } - - return this.state.isEditMode ? ( - - ) : ( - - - {title.label || defaultLabel} -   - - - - ); - } -} -AdhocMetricEditPopoverTitle.propTypes = propTypes; diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.jsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.jsx deleted file mode 100644 index dd2b007bf91b..000000000000 --- a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.jsx +++ /dev/null @@ -1,70 +0,0 @@ -/** - * 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. - */ -/* eslint-disable no-unused-expressions */ -import React from 'react'; -import sinon from 'sinon'; -import { shallow } from 'enzyme'; -import { Tooltip } from 'src/components/Tooltip'; - -import AdhocMetricEditPopoverTitle from 'src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle'; - -const title = { - label: 'Title', - hasCustomLabel: false, -}; - -function setup(overrides) { - const onChange = sinon.spy(); - const props = { - title, - onChange, - ...overrides, - }; - const wrapper = shallow(); - return { wrapper, onChange }; -} - -describe('AdhocMetricEditPopoverTitle', () => { - it('renders an OverlayTrigger wrapper with the title', () => { - const { wrapper } = setup(); - expect(wrapper.find(Tooltip)).toExist(); - expect( - wrapper.find('[data-test="AdhocMetricEditTitle#trigger"]').text(), - ).toBe(`${title.label}\xa0`); - }); - - it('transfers to edit mode when clicked', () => { - const { wrapper } = setup(); - expect(wrapper.state('isEditMode')).toBe(false); - wrapper - .find('[data-test="AdhocMetricEditTitle#trigger"]') - .simulate('click'); - expect(wrapper.state('isEditMode')).toBe(true); - }); - - it('Render non-interactive span with title when edit is disabled', () => { - const { wrapper } = setup({ isEditDisabled: true }); - expect( - wrapper.find('[data-test="AdhocMetricTitle"]').exists(), - ).toBeTruthy(); - expect( - wrapper.find('[data-test="AdhocMetricEditTitle#trigger"]').exists(), - ).toBeFalsy(); - }); -}); diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.tsx new file mode 100644 index 000000000000..a91e0cac6f5a --- /dev/null +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.test.tsx @@ -0,0 +1,141 @@ +/** + * 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 React from 'react'; +import userEvent from '@testing-library/user-event'; +import { + screen, + render, + fireEvent, + waitFor, +} from 'spec/helpers/testing-library'; + +import AdhocMetricEditPopoverTitle, { + AdhocMetricEditPopoverTitleProps, +} from 'src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle'; + +const titleProps = { + label: 'COUNT(*)', + hasCustomLabel: false, +}; + +const setup = (props: Partial = {}) => { + const onChange = jest.fn(); + + const { container } = render( + , + ); + + return { container, onChange }; +}; + +test('should render', async () => { + const { container } = setup(); + expect(container).toBeInTheDocument(); + + expect(screen.queryByTestId('AdhocMetricTitle')).not.toBeInTheDocument(); + expect(screen.getByText(titleProps.label)).toBeVisible(); +}); + +test('should render tooltip on hover', async () => { + const { container } = setup(); + + expect(screen.queryByText('Click to edit label')).not.toBeInTheDocument(); + fireEvent.mouseOver(screen.getByTestId('AdhocMetricEditTitle#trigger')); + + expect(await screen.findByText('Click to edit label')).toBeInTheDocument(); + expect( + container.parentElement?.getElementsByClassName('ant-tooltip-hidden') + .length, + ).toBe(0); + + fireEvent.mouseOut(screen.getByTestId('AdhocMetricEditTitle#trigger')); + await waitFor(() => { + expect( + container.parentElement?.getElementsByClassName('ant-tooltip-hidden') + .length, + ).toBe(1); + }); +}); + +test('render non-interactive span with title when edit is disabled', async () => { + const { container } = setup({ isEditDisabled: true }); + expect(container).toBeInTheDocument(); + + expect(screen.queryByTestId('AdhocMetricTitle')).toBeInTheDocument(); + expect(screen.getByText(titleProps.label)).toBeVisible(); + expect( + screen.queryByTestId('AdhocMetricEditTitle#trigger'), + ).not.toBeInTheDocument(); +}); + +test('render default label if no title is provided', async () => { + const { container } = setup({ title: undefined }); + expect(container).toBeInTheDocument(); + + expect(screen.queryByTestId('AdhocMetricTitle')).not.toBeInTheDocument(); + expect(screen.getByText('My metric')).toBeVisible(); +}); + +test('start and end the title edit mode', async () => { + const { container, onChange } = setup(); + expect(container).toBeInTheDocument(); + + expect(container.getElementsByTagName('i')[0]).toBeVisible(); + expect(screen.getByText(titleProps.label)).toBeVisible(); + expect( + screen.queryByTestId('AdhocMetricEditTitle#input'), + ).not.toBeInTheDocument(); + + fireEvent.click( + container.getElementsByClassName('AdhocMetricEditPopoverTitle')[0], + ); + + expect(await screen.findByTestId('AdhocMetricEditTitle#input')).toBeVisible(); + userEvent.type(screen.getByTestId('AdhocMetricEditTitle#input'), 'Test'); + + expect(onChange).toHaveBeenCalledTimes(4); + fireEvent.keyPress(screen.getByTestId('AdhocMetricEditTitle#input'), { + key: 'Enter', + charCode: 13, + }); + + expect( + screen.queryByTestId('AdhocMetricEditTitle#input'), + ).not.toBeInTheDocument(); + + fireEvent.click( + container.getElementsByClassName('AdhocMetricEditPopoverTitle')[0], + ); + + expect(await screen.findByTestId('AdhocMetricEditTitle#input')).toBeVisible(); + userEvent.type( + screen.getByTestId('AdhocMetricEditTitle#input'), + 'Second test', + ); + expect(onChange).toHaveBeenCalled(); + + fireEvent.blur(screen.getByTestId('AdhocMetricEditTitle#input')); + expect( + screen.queryByTestId('AdhocMetricEditTitle#input'), + ).not.toBeInTheDocument(); +}); diff --git a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx new file mode 100644 index 000000000000..da6a2739c387 --- /dev/null +++ b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.tsx @@ -0,0 +1,127 @@ +/** + * 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 React, { + ChangeEventHandler, + FocusEvent, + KeyboardEvent, + useCallback, + useState, +} from 'react'; +import { t, styled } from '@superset-ui/core'; +import { Input } from 'src/components/Input'; +import { Tooltip } from 'src/components/Tooltip'; + +const TitleLabel = styled.span` + display: inline-block; + padding: 2px 0; +`; + +const StyledInput = styled(Input)` + border-radius: ${({ theme }) => theme.borderRadius}; + height: 26px; + padding-left: ${({ theme }) => theme.gridUnit * 2.5}px; +`; + +export interface AdhocMetricEditPopoverTitleProps { + title?: { + label?: string; + hasCustomLabel?: boolean; + }; + isEditDisabled?: boolean; + onChange: ChangeEventHandler; +} + +const AdhocMetricEditPopoverTitle: React.FC = + ({ title, isEditDisabled, onChange }) => { + const [isHovered, setIsHovered] = useState(false); + const [isEditMode, setIsEditMode] = useState(false); + + const defaultLabel = t('My metric'); + + const handleMouseOver = useCallback(() => setIsHovered(true), []); + const handleMouseOut = useCallback(() => setIsHovered(false), []); + const handleClick = useCallback(() => setIsEditMode(true), []); + const handleBlur = useCallback(() => setIsEditMode(false), []); + + const handleKeyPress = useCallback( + (ev: KeyboardEvent) => { + if (ev.key === 'Enter') { + ev.preventDefault(); + handleBlur(); + } + }, + [handleBlur], + ); + + const handleInputBlur = useCallback( + (e: FocusEvent) => { + if (e.target.value === '') { + onChange(e); + } + + handleBlur(); + }, + [onChange, handleBlur], + ); + + if (isEditDisabled) { + return ( + {title?.label || defaultLabel} + ); + } + + if (isEditMode) { + return ( + + ); + } + + return ( + + + {title?.label || defaultLabel} +   + + + + ); + }; + +export default AdhocMetricEditPopoverTitle;