diff --git a/superset-frontend/src/common/components/Modal/Modal.tsx b/superset-frontend/src/common/components/Modal/Modal.tsx index d81dc850335..3ab5a9350c7 100644 --- a/superset-frontend/src/common/components/Modal/Modal.tsx +++ b/superset-frontend/src/common/components/Modal/Modal.tsx @@ -42,6 +42,7 @@ interface ModalProps { footer?: React.ReactNode; wrapProps?: object; height?: string; + closable?: boolean; } interface StyledModalProps extends SupersetThemeProps { diff --git a/superset-frontend/src/components/OmniContainer.jsx b/superset-frontend/src/components/OmniContainer.jsx deleted file mode 100644 index e641a7c3af3..00000000000 --- a/superset-frontend/src/components/OmniContainer.jsx +++ /dev/null @@ -1,102 +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 PropTypes from 'prop-types'; -import { styled, t, SupersetClient } from '@superset-ui/core'; -import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; -import Omnibar from 'omnibar'; -import Modal from 'src/common/components/Modal'; -import { LOG_ACTIONS_OMNIBAR_TRIGGERED } from '../logger/LogUtils'; - -const propTypes = { - logEvent: PropTypes.func.isRequired, -}; - -const getDashboards = query => - // todo: Build a dedicated endpoint for dashboard searching - // i.e. superset/v1/api/dashboards?q=${query} - SupersetClient.get({ - endpoint: `/dashboardasync/api/read?_oc_DashboardModelViewAsync=changed_on&_od_DashboardModelViewAsync=desc&_flt_2_dashboard_title=${query}`, - }) - .then(({ json }) => - json.result.map(item => ({ - title: item.dashboard_title, - ...item, - })), - ) - .catch(() => ({ - title: t('An error occurred while fetching dashboards'), - })); - -const OmniModal = styled(Modal)` - margin-top: 20%; - - .ant-modal-body { - padding: 0; - } -`; - -class OmniContainer extends React.Component { - constructor(props) { - super(props); - this.state = { - showOmni: false, - }; - this.handleKeydown = this.handleKeydown.bind(this); - } - - componentDidMount() { - document.addEventListener('keydown', this.handleKeydown); - } - - componentWillUnmount() { - document.removeEventListener('keydown', this.handleKeydown); - } - - handleKeydown(event) { - const controlOrCommand = event.ctrlKey || event.metaKey; - if (controlOrCommand && isFeatureEnabled(FeatureFlag.OMNIBAR)) { - const isK = event.key === 'k' || event.keyCode === 83; - if (isK) { - this.props.logEvent(LOG_ACTIONS_OMNIBAR_TRIGGERED, { - show_omni: !this.state.showOmni, - }); - - this.setState(prevState => ({ showOmni: !prevState.showOmni })); - - document.getElementsByClassName('Omnibar')[0].focus(); - } - } - } - - render() { - return ( - - - - ); - } -} - -OmniContainer.propTypes = propTypes; -export default OmniContainer; diff --git a/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx b/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx new file mode 100644 index 00000000000..0e46993f45e --- /dev/null +++ b/superset-frontend/src/components/OmniContainer/OmniContainer.test.tsx @@ -0,0 +1,187 @@ +/** + * 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 { render, screen, fireEvent } from 'spec/helpers/testing-library'; +import { isFeatureEnabled } from 'src/featureFlags'; +import OmniContainer from './index'; + +jest.mock('src/featureFlags', () => ({ + isFeatureEnabled: jest.fn(), + FeatureFlag: { OMNIBAR: 'OMNIBAR' }, +})); + +test('Do not open Omnibar with the featureflag disabled', () => { + (isFeatureEnabled as jest.Mock).mockImplementation( + (ff: string) => !(ff === 'OMNIBAR'), + ); + const logEvent = jest.fn(); + render( +
+ +
, + ); + + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); + fireEvent.keyDown(screen.getByTestId('test'), { + ctrlKey: true, + code: 'KeyK', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); +}); + +test('Open Omnibar with ctrl + k with featureflag enabled', () => { + (isFeatureEnabled as jest.Mock).mockImplementation( + (ff: string) => ff === 'OMNIBAR', + ); + const logEvent = jest.fn(); + render( +
+ +
, + ); + + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); + + // show Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + ctrlKey: true, + code: 'KeyK', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).toBeInTheDocument(); + + // hide Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + ctrlKey: true, + code: 'KeyK', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeVisible(); +}); + +test('Open Omnibar with ctrl + s with featureflag enabled', () => { + (isFeatureEnabled as jest.Mock).mockImplementation( + (ff: string) => ff === 'OMNIBAR', + ); + const logEvent = jest.fn(); + render( +
+ +
, + ); + + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); + + // show Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + ctrlKey: true, + code: 'KeyS', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).toBeInTheDocument(); + + // hide Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + ctrlKey: true, + code: 'KeyS', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeVisible(); +}); + +test('Open Omnibar with Command + k with featureflag enabled', () => { + (isFeatureEnabled as jest.Mock).mockImplementation( + (ff: string) => ff === 'OMNIBAR', + ); + const logEvent = jest.fn(); + render( +
+ +
, + ); + + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); + + // show Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + metaKey: true, + code: 'KeyK', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).toBeInTheDocument(); + + // hide Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + metaKey: true, + code: 'KeyK', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeVisible(); +}); + +test('Open Omnibar with Command + s with featureflag enabled', () => { + (isFeatureEnabled as jest.Mock).mockImplementation( + (ff: string) => ff === 'OMNIBAR', + ); + const logEvent = jest.fn(); + render( +
+ +
, + ); + + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeInTheDocument(); + + // show Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + metaKey: true, + code: 'KeyS', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).toBeInTheDocument(); + + // hide Omnibar + fireEvent.keyDown(screen.getByTestId('test'), { + metaKey: true, + code: 'KeyS', + }); + expect( + screen.queryByPlaceholderText('Search all dashboards'), + ).not.toBeVisible(); +}); diff --git a/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx b/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx new file mode 100644 index 00000000000..39b367326a1 --- /dev/null +++ b/superset-frontend/src/components/OmniContainer/Omnibar.test.tsx @@ -0,0 +1,38 @@ +/** + * 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 { render, screen } from 'spec/helpers/testing-library'; +import { Omnibar } from './Omnibar'; + +test('Must put id on input', () => { + render( + , + ); + + expect(screen.getByPlaceholderText('Test Omnibar')).toBeInTheDocument(); + expect(screen.getByPlaceholderText('Test Omnibar')).toHaveAttribute( + 'id', + 'test-id-attribute', + ); +}); diff --git a/superset-frontend/src/components/OmniContainer/Omnibar.tsx b/superset-frontend/src/components/OmniContainer/Omnibar.tsx new file mode 100644 index 00000000000..2cba1c99a56 --- /dev/null +++ b/superset-frontend/src/components/OmniContainer/Omnibar.tsx @@ -0,0 +1,44 @@ +/** + * 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 OmnibarDeprecated from 'omnibar'; + +interface Props { + id: string; + placeholder: string; + extensions: ((query: string) => Promise)[]; +} + +/** + * @deprecated Component "omnibar" does not support prop className or id (the original implementation used className). However, the original javascript code was sending these prop and was working correctly. lol + * As this behavior is unpredictable, and does not works whitch types, I have isolated this component so that in the future a better solution can be found and implemented. + * We need to find a substitute for this component or some way of working around this problem + */ +export function Omnibar({ extensions, placeholder, id }: Props) { + return ( + + ); +} diff --git a/superset-frontend/src/components/OmniContainer/getDashboards.ts b/superset-frontend/src/components/OmniContainer/getDashboards.ts new file mode 100644 index 00000000000..faa8336b0bc --- /dev/null +++ b/superset-frontend/src/components/OmniContainer/getDashboards.ts @@ -0,0 +1,54 @@ +/** + * 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 { t, SupersetClient } from '@superset-ui/core'; + +interface DashboardItem { + changed_by_name: string; + changed_on: string; + creator: string; + dashboard_link: string; + dashboard_title: string; + id: number; + modified: string; + url: string; +} + +interface Dashboards extends DashboardItem { + title: string; +} + +export const getDashboards = async ( + query: string, +): Promise<(Dashboards | { title: string })[]> => { + // todo: Build a dedicated endpoint for dashboard searching + // i.e. superset/v1/api/dashboards?q=${query} + let response; + try { + response = await SupersetClient.get({ + endpoint: `/dashboardasync/api/read?_oc_DashboardModelViewAsync=changed_on&_od_DashboardModelViewAsync=desc&_flt_2_dashboard_title=${query}`, + }); + } catch (error) { + return [{ title: t('An error occurred while fetching dashboards') }]; + } + return response?.json.result.map((item: DashboardItem) => ({ + title: item.dashboard_title, + ...item, + })); +}; diff --git a/superset-frontend/src/components/OmniContainer/index.tsx b/superset-frontend/src/components/OmniContainer/index.tsx new file mode 100644 index 00000000000..a9b90d06d1a --- /dev/null +++ b/superset-frontend/src/components/OmniContainer/index.tsx @@ -0,0 +1,82 @@ +/** + * 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, { useRef, useState } from 'react'; +import { styled } from '@superset-ui/core'; +import { isFeatureEnabled, FeatureFlag } from 'src/featureFlags'; +import Modal from 'src/common/components/Modal'; +import { useComponentDidMount } from 'src/common/hooks/useComponentDidMount'; +import { Omnibar } from './Omnibar'; +import { LOG_ACTIONS_OMNIBAR_TRIGGERED } from '../../logger/LogUtils'; +import { getDashboards } from './getDashboards'; + +const OmniModal = styled(Modal)` + margin-top: 20%; + + .ant-modal-body { + padding: 0; + } +`; + +interface Props { + logEvent: (log: string, object: object) => void; +} + +export default function OmniContainer({ logEvent }: Props) { + const showOmni = useRef(); + const [showModal, setShowModal] = useState(false); + + useComponentDidMount(() => { + showOmni.current = false; + function handleKeydown(event: KeyboardEvent) { + if (!isFeatureEnabled(FeatureFlag.OMNIBAR)) return; + const controlOrCommand = event.ctrlKey || event.metaKey; + const isOk = ['KeyK', 'KeyS'].includes(event.code); // valid keys "s" or "k" + if (controlOrCommand && isOk) { + logEvent(LOG_ACTIONS_OMNIBAR_TRIGGERED, { + show_omni: !!showOmni.current, + }); + showOmni.current = !showOmni.current; + setShowModal(showOmni.current); + if (showOmni.current) { + document.getElementById('InputOmnibar')?.focus(); + } + } + } + + document.addEventListener('keydown', handleKeydown); + return () => document.removeEventListener('keydown', handleKeydown); + }); + + return ( + {}} + > + + + ); +}