Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate AddToDashboard dialog #4408

Merged
merged 5 commits into from
Dec 4, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/app/components/app-header/AppHeader.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function DesktopNavbar() {
)}
{currentUser.hasPermission('create_dashboard') && (
<Menu.Item key="new-dashboard">
<a onMouseUp={CreateDashboardDialog.showModal}>New Dashboard</a>
<a onMouseUp={() => CreateDashboardDialog.showModal()}>New Dashboard</a>
</Menu.Item>
)}
<Menu.Item key="new-alert">
Expand Down
110 changes: 110 additions & 0 deletions client/app/components/queries/AddToDashboardDialog.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import { isString } from 'lodash';
import React, { useState, useEffect } from 'react';
import PropTypes from 'prop-types';
import Modal from 'antd/lib/modal';
import Input from 'antd/lib/input';
import List from 'antd/lib/list';
import Icon from 'antd/lib/icon';
import { wrap as wrapDialog, DialogPropType } from '@/components/DialogWrapper';
import { QueryTagsControl } from '@/components/tags-control/TagsControl';
import { Dashboard } from '@/services/dashboard';
import notification from '@/services/notification';
import useSearchResults from '@/lib/hooks/useSearchResults';

import './add-to-dashboard-dialog.less';

function AddToDashboardDialog({ dialog, visualization }) {
const [searchTerm, setSearchTerm] = useState('');

const [doSearch, dashboards, isLoading] = useSearchResults((term) => {
if (isString(term) && (term.length >= 3)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loading indicator was not the only thing that caused me a bit of confusion 😕, do we really need this term.length >= 3? (I mean, the results are paged, right?) There is the option of using Antd form to indicate that minimum length, but I don't think it's that worth it. Well, wdyt?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why it was implemented this way; of course that API can search by a single character. Also, here we don't show all results, but only a most relevant ones (20 or 25 - don't remember exactly) - I think it makes sense.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the idea was to reduce number of requests we send because it's less likely that a <3 characters term will be meaningful. We can drop this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return Dashboard.get({ q: term }).$promise.then(results => results.results);
}
return Promise.resolve([]);
}, { initialResults: [] });

const [selectedDashboard, setSelectedDashboard] = useState(null);

const [saveInProgress, setSaveInProgress] = useState(false);

useEffect(() => { doSearch(searchTerm); }, [doSearch, searchTerm]);

function addWidgetToDashboard() {
// Load dashboard with all widgets
Dashboard.get({ slug: selectedDashboard.slug }).$promise
.then((dashboard) => {
dashboard.addWidget(visualization);
return dashboard;
})
.then((dashboard) => {
dialog.close();
const key = `notification-${Math.random().toString(36).substr(2, 10)}`;
notification.success('Widget added to dashboard', (
<React.Fragment>
<a href={`dashboard/${dashboard.slug}`} onClick={() => notification.close(key)}>{dashboard.name}</a>
<QueryTagsControl isDraft={dashboard.is_draft} tags={dashboard.tags} />
</React.Fragment>
), { key });
})
.catch(() => { notification.error('Widget not added.'); })
.finally(() => { setSaveInProgress(false); });
}

const items = selectedDashboard ? [selectedDashboard] : dashboards;

return (
<Modal
{...dialog.props}
title="Add to Dashboard"
okButtonProps={{ disabled: !selectedDashboard || saveInProgress, loading: saveInProgress }}
cancelButtonProps={{ disabled: saveInProgress }}
onOk={addWidgetToDashboard}
>
<label htmlFor="add-to-dashboard-dialog-dashboard">Choose the dashboard to add this query to:</label>

{!selectedDashboard && (
<Input
id="add-to-dashboard-dialog-dashboard"
className="w-100"
autoComplete="off"
autoFocus
placeholder="Search a dashboard by name"
value={searchTerm}
onChange={event => setSearchTerm(event.target.value)}
suffix={(
<Icon type="close" className={(searchTerm === '') ? 'hidden' : null} onClick={() => setSearchTerm('')} />
)}
/>
)}

{((items.length > 0) || isLoading) && (
<List
className={selectedDashboard ? 'add-to-dashboard-dialog-selection' : 'add-to-dashboard-dialog-search-results'}
bordered
itemLayout="horizontal"
loading={isLoading}
dataSource={items}
renderItem={d => (
<List.Item
key={`dashboard-${d.id}`}
actions={selectedDashboard ? [<Icon type="close" onClick={() => setSelectedDashboard(null)} />] : []}
onClick={selectedDashboard ? null : () => setSelectedDashboard(d)}
>
<div className="add-to-dashboard-dialog-item-content">
{d.name}
<QueryTagsControl isDraft={d.is_draft} tags={d.tags} />
</div>
</List.Item>
)}
/>
)}
</Modal>
);
}

AddToDashboardDialog.propTypes = {
dialog: DialogPropType.isRequired,
visualization: PropTypes.object.isRequired, // eslint-disable-line react/forbid-prop-types
};

export default wrapDialog(AddToDashboardDialog);
37 changes: 37 additions & 0 deletions client/app/components/queries/add-to-dashboard-dialog.less
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
@import (reference, less) '~@/assets/less/main.less';

.ant-list {
&.add-to-dashboard-dialog-search-results {
margin-top: 15px;

.ant-list-items {
max-height: 300px;
overflow: auto;
}

.ant-list-item {
padding: 12px;
cursor: pointer;

&:hover, &:active {
@table-row-hover-bg: fade(@redash-gray, 5%);
background-color: @table-row-hover-bg;
}
}
}

&.add-to-dashboard-dialog-selection {
.ant-list-item {
padding: 12px;

.add-to-dashboard-dialog-item-content {
flex: 1 1 auto;
}

.ant-list-item-action li {
margin: 0;
padding: 0;
}
}
}
}
33 changes: 33 additions & 0 deletions client/app/lib/hooks/useSearchResults.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { useState, useEffect } from 'react';
import { useDebouncedCallback } from 'use-debounce';

export default function useSearchResults(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool 👍, I will keep it in mind for next search uses

fetch,
{ initialResults = null, debounceTimeout = 200 } = {},
) {
const [result, setResult] = useState(initialResults);
const [isLoading, setIsLoading] = useState(false);

let currentSearchTerm = null;
let isDestroyed = false;

const [doSearch] = useDebouncedCallback((searchTerm) => {
setIsLoading(true);
currentSearchTerm = searchTerm;
fetch(searchTerm)
.catch(() => null)
.then((data) => {
if ((searchTerm === currentSearchTerm) && !isDestroyed) {
setResult(data);
setIsLoading(false);
}
});
}, debounceTimeout);

useEffect(() => (
// ignore all requests after component destruction
() => { isDestroyed = true; }
), []);

return [doSearch, result, isLoading];
}
19 changes: 0 additions & 19 deletions client/app/pages/queries/add-to-dashboard.html

This file was deleted.

56 changes: 0 additions & 56 deletions client/app/pages/queries/add-to-dashboard.js

This file was deleted.

11 changes: 3 additions & 8 deletions client/app/pages/queries/view.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import ScheduleDialog from '@/components/queries/ScheduleDialog';
import { newVisualization } from '@/visualizations';
import EditVisualizationDialog from '@/visualizations/EditVisualizationDialog';
import EmbedQueryDialog from '@/components/queries/EmbedQueryDialog';
import AddToDashboardDialog from '@/components/queries/AddToDashboardDialog';
import PermissionsEditorDialog from '@/components/permissions-editor/PermissionsEditorDialog';
import notification from '@/services/notification';
import template from './query.html';
Expand Down Expand Up @@ -499,14 +500,8 @@ function QueryViewCtrl(
};

$scope.openAddToDashboardForm = (visId) => {
const visualization = getVisualization(visId);
$uibModal.open({
component: 'addToDashboardDialog',
size: 'sm',
resolve: {
query: $scope.query,
vis: visualization,
},
AddToDashboardDialog.showModal({
visualization: getVisualization(visId),
});
};

Expand Down