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 5bfc95e79e feat: When editing the label/title in the Metrics popover,
hitting Enter should save what you've typed (#19898)
5bfc95e79e is described below
commit 5bfc95e79e89961967ba4acc8d24131157ccd16b
Author: Diego Medina <[email protected]>
AuthorDate: Wed Jun 8 18:52:53 2022 -0400
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 +-
.../MetricControl/AdhocMetricEditPopoverTitle.jsx | 115 -----------------
.../AdhocMetricEditPopoverTitle.test.jsx | 70 ----------
.../AdhocMetricEditPopoverTitle.test.tsx | 141 +++++++++++++++++++++
.../MetricControl/AdhocMetricEditPopoverTitle.tsx | 127 +++++++++++++++++++
6 files changed, 276 insertions(+), 194 deletions(-)
diff --git a/superset-frontend/src/assets/stylesheets/superset.less
b/superset-frontend/src/assets/stylesheets/superset.less
index 0cf419b30d..5808d0144b 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 eecce0b333..b50abb9aae 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 ? (
- <Input
- className="metric-edit-popover-label-input"
+ <StyledInput
type="text"
placeholder={title}
value={hasCustomLabel ? title : ''}
diff --git
a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.jsx
b/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.jsx
deleted file mode 100644
index cef62505e4..0000000000
---
a/superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopoverTitle.jsx
+++ /dev/null
@@ -1,115 +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.
- */
-import React from 'react';
-import { t } from '@superset-ui/core';
-import PropTypes from 'prop-types';
-import { Input } from 'src/components/Input';
-import { Tooltip } from 'src/components/Tooltip';
-
-const propTypes = {
- title: PropTypes.shape({
- label: PropTypes.string,
- hasCustomLabel: PropTypes.bool,
- }),
- onChange: PropTypes.func.isRequired,
- isEditDisabled: PropTypes.bool,
-};
-
-export default class AdhocMetricEditPopoverTitle extends React.Component {
- constructor(props) {
- super(props);
- this.onMouseOver = this.onMouseOver.bind(this);
- this.onMouseOut = this.onMouseOut.bind(this);
- this.onClick = this.onClick.bind(this);
- this.onBlur = this.onBlur.bind(this);
- this.onInputBlur = this.onInputBlur.bind(this);
- this.state = {
- isHovered: false,
- isEditMode: false,
- };
- }
-
- onMouseOver() {
- this.setState({ isHovered: true });
- }
-
- onMouseOut() {
- this.setState({ isHovered: false });
- }
-
- onClick() {
- this.setState({ isEditMode: true });
- }
-
- onBlur() {
- this.setState({ isEditMode: false });
- }
-
- onInputBlur(e) {
- if (e.target.value === '') {
- this.props.onChange(e);
- }
- this.onBlur();
- }
-
- render() {
- const { title, onChange, isEditDisabled } = this.props;
- const defaultLabel = t('My metric');
-
- if (isEditDisabled) {
- return (
- <span data-test="AdhocMetricTitle">{title.label || defaultLabel}</span>
- );
- }
-
- return this.state.isEditMode ? (
- <Input
- className="metric-edit-popover-label-input"
- type="text"
- placeholder={title.label}
- value={title.hasCustomLabel ? title.label : ''}
- autoFocus
- onChange={onChange}
- onBlur={this.onInputBlur}
- data-test="AdhocMetricEditTitle#input"
- />
- ) : (
- <Tooltip placement="top" title="Click to edit label">
- <span
- className="AdhocMetricEditPopoverTitle inline-editable"
- data-test="AdhocMetricEditTitle#trigger"
- onMouseOver={this.onMouseOver}
- onMouseOut={this.onMouseOut}
- onClick={this.onClick}
- onBlur={this.onBlur}
- role="button"
- tabIndex={0}
- >
- {title.label || defaultLabel}
-
- <i
- className="fa fa-pencil"
- style={{ color: this.state.isHovered ? 'black' : 'grey' }}
- />
- </span>
- </Tooltip>
- );
- }
-}
-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 dd2b007bf9..0000000000
---
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(<AdhocMetricEditPopoverTitle {...props} />);
- 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 0000000000..a91e0cac6f
--- /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<AdhocMetricEditPopoverTitleProps> = {}) => {
+ const onChange = jest.fn();
+
+ const { container } = render(
+ <AdhocMetricEditPopoverTitle
+ title={titleProps}
+ onChange={onChange}
+ {...props}
+ />,
+ );
+
+ 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 0000000000..da6a2739c3
--- /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<HTMLInputElement>;
+}
+
+const AdhocMetricEditPopoverTitle: React.FC<AdhocMetricEditPopoverTitleProps> =
+ ({ 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<HTMLInputElement>) => {
+ if (ev.key === 'Enter') {
+ ev.preventDefault();
+ handleBlur();
+ }
+ },
+ [handleBlur],
+ );
+
+ const handleInputBlur = useCallback(
+ (e: FocusEvent<HTMLInputElement>) => {
+ if (e.target.value === '') {
+ onChange(e);
+ }
+
+ handleBlur();
+ },
+ [onChange, handleBlur],
+ );
+
+ if (isEditDisabled) {
+ return (
+ <span data-test="AdhocMetricTitle">{title?.label ||
defaultLabel}</span>
+ );
+ }
+
+ if (isEditMode) {
+ return (
+ <StyledInput
+ type="text"
+ placeholder={title?.label}
+ value={title?.hasCustomLabel ? title.label : ''}
+ autoFocus
+ onChange={onChange}
+ onBlur={handleInputBlur}
+ onKeyPress={handleKeyPress}
+ data-test="AdhocMetricEditTitle#input"
+ />
+ );
+ }
+
+ return (
+ <Tooltip placement="top" title="Click to edit label">
+ <span
+ className="AdhocMetricEditPopoverTitle inline-editable"
+ data-test="AdhocMetricEditTitle#trigger"
+ onMouseOver={handleMouseOver}
+ onMouseOut={handleMouseOut}
+ onClick={handleClick}
+ onBlur={handleBlur}
+ role="button"
+ tabIndex={0}
+ >
+ <TitleLabel>{title?.label || defaultLabel}</TitleLabel>
+
+ <i
+ className="fa fa-pencil"
+ style={{ color: isHovered ? 'black' : 'grey' }}
+ />
+ </span>
+ </Tooltip>
+ );
+ };
+
+export default AdhocMetricEditPopoverTitle;
