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.

Sign up for GitHub

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 Query Source page to React: Editor area #4468

Merged
merged 16 commits into from
Dec 24, 2019
Merged

Migrate Query Source page to React: Editor area #4468

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e53e10c
Migrate Query Source page to React: Query Editor section (top-right)
kravets-levko Dec 20, 2019
2cdefa2
Integration with SchemaBrowser
kravets-levko Dec 20, 2019
86821a6
Add Parameter dialog + update parameters on query source change
kravets-levko Dec 20, 2019
686c498
Code style
kravets-levko Dec 20, 2019
653dd3f
Properly clone query object on updates
kravets-levko Dec 20, 2019
d4a8426
Merge branch 'migrate-query-source-view-to-react' into migrate-query-…
kravets-levko Dec 23, 2019
126eeba
Implement Execute query button
kravets-levko Dec 23, 2019
f8631c1
Fix crash when query (new) has no visualizations yet
kravets-levko Dec 23, 2019
14e0b36
Fix Execute query bugs
kravets-levko Dec 23, 2019
d857074
Keyboard shortcuts; handle parameters changes
kravets-levko Dec 23, 2019
1157bbb
Improve useQueryExecute hook
kravets-levko Dec 23, 2019
63c24f9
Only bind execute keys when possible to execute
gabrieldutra Dec 23, 2019
afcd73d
Update dirtyParameters with parameter changes
gabrieldutra Dec 23, 2019
8cf3e39
Move recordEvent to useQueryExecute
gabrieldutra Dec 23, 2019
16057bf
Refine shortcuts binding
kravets-levko Dec 23, 2019
57bf864
Show only primary shortcut
kravets-levko Dec 24, 2019
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
59 changes: 49 additions & 10 deletions client/app/components/queries/QueryEditor/QueryEditorControls.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,37 @@
import { map } from "lodash";
import React from "react";
import { isFunction, map, filter, fromPairs } from "lodash";
import React, { useEffect } from "react";
import PropTypes from "prop-types";
import Tooltip from "antd/lib/tooltip";
import Button from "antd/lib/button";
import Select from "antd/lib/select";
import { KeyboardShortcuts, humanReadableShortcut } from "@/services/keyboard-shortcuts";

import AutocompleteToggle from "./AutocompleteToggle";
import "./QueryEditorControls.less";

function ButtonTooltip({ title, shortcut, ...props }) {
shortcut = humanReadableShortcut(shortcut);
title =
title && shortcut ? (
<React.Fragment>
{title} (<i>{shortcut}</i>)
</React.Fragment>
) : (
title || shortcut
);
return <Tooltip placement="top" title={title} {...props} />;
}

ButtonTooltip.propTypes = {
title: PropTypes.node,
shortcut: PropTypes.string,
};

ButtonTooltip.defaultProps = {
title: null,
shortcut: null,
};

export default function EditorControl({
addParameterButtonProps,
formatButtonProps,
Expand All @@ -16,28 +40,42 @@ export default function EditorControl({
autocompleteToggleProps,
dataSourceSelectorProps,
}) {
useEffect(() => {
const buttons = filter(
[addParameterButtonProps, formatButtonProps, saveButtonProps, executeButtonProps],
b => b.shortcut && !b.disabled && isFunction(b.onClick)
);
if (buttons.length > 0) {
const shortcuts = fromPairs(map(buttons, b => [b.shortcut, b.onClick]));
KeyboardShortcuts.bind(shortcuts);
return () => {
KeyboardShortcuts.unbind(shortcuts);
};
}
}, [addParameterButtonProps, formatButtonProps, saveButtonProps, executeButtonProps]);

return (
<div className="query-editor-controls">
{addParameterButtonProps !== false && (
<Tooltip placement="top" title={addParameterButtonProps.title}>
<ButtonTooltip title={addParameterButtonProps.title} shortcut={addParameterButtonProps.shortcut}>
<Button
className="query-editor-controls-button m-r-5"
disabled={addParameterButtonProps.disabled}
onClick={addParameterButtonProps.onClick}>
{"{{"}&nbsp;{"}}"}
</Button>
</Tooltip>
</ButtonTooltip>
)}
{formatButtonProps !== false && (
<Tooltip placement="top" title={formatButtonProps.title}>
<ButtonTooltip title={formatButtonProps.title} shortcut={formatButtonProps.shortcut}>
<Button
className="query-editor-controls-button m-r-5"
disabled={formatButtonProps.disabled}
onClick={formatButtonProps.onClick}>
<span className="zmdi zmdi-format-indent-increase" />
{formatButtonProps.text}
</Button>
</Tooltip>
</ButtonTooltip>
)}
{autocompleteToggleProps !== false && (
<AutocompleteToggle
Expand All @@ -61,7 +99,7 @@ export default function EditorControl({
</Select>
)}
{saveButtonProps !== false && (
<Tooltip placement="top" title={saveButtonProps.title}>
<ButtonTooltip title={saveButtonProps.title} shortcut={saveButtonProps.shortcut}>
<Button
className="query-editor-controls-button m-l-5"
disabled={saveButtonProps.disabled}
Expand All @@ -70,10 +108,10 @@ export default function EditorControl({
<span className="fa fa-floppy-o" />
{saveButtonProps.text}
</Button>
</Tooltip>
</ButtonTooltip>
)}
{executeButtonProps !== false && (
<Tooltip placement="top" title={executeButtonProps.title}>
<ButtonTooltip title={executeButtonProps.title} shortcut={executeButtonProps.shortcut}>
<Button
className="query-editor-controls-button m-l-5"
type="primary"
Expand All @@ -83,7 +121,7 @@ export default function EditorControl({
<span className="zmdi zmdi-play" />
{executeButtonProps.text}
</Button>
</Tooltip>
</ButtonTooltip>
)}
</div>
);
Expand All @@ -96,6 +134,7 @@ const ButtonPropsPropType = PropTypes.oneOfType([
disabled: PropTypes.bool,
onClick: PropTypes.func,
text: PropTypes.node,
shortcut: PropTypes.string,
}),
]);

Expand Down
60 changes: 14 additions & 46 deletions client/app/pages/queries/QuerySource.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { Query } from "@/services/query";
import { DataSource, SCHEMA_NOT_SUPPORTED } from "@/services/data-source";
import notification from "@/services/notification";
import recordEvent from "@/services/recordEvent";
import { KeyboardShortcuts } from "@/services/keyboard-shortcuts";
import navigateTo from "@/services/navigateTo";
import localOptions from "@/lib/localOptions";

Expand Down Expand Up @@ -254,49 +253,24 @@ function QuerySource(props) {

const canExecuteQuery = useMemo(
() =>
!isEmpty(query.query) &&
!isQueryExecuting &&
!dirtyParameters &&
(query.is_safe || (currentUser.hasPermission("execute_query") && dataSource && !dataSource.view_only)),
[dirtyParameters, query, dataSource]
[isQueryExecuting, dirtyParameters, query, dataSource]
);
const isDirty = query.query !== originalQuerySource;

const doExecuteQuery = useCallback(() => {
if (!canExecuteQuery) {
return;
}
if (isDirty || !isEmpty(selectedText)) {
executeAdhocQuery(selectedText);
} else {
executeQuery();
}
}, [isDirty, selectedText, executeQuery, executeAdhocQuery]);

useEffect(() => {
const shortcuts = {
"mod+s": saveQuery,
"mod+p": openAddNewParameterDialog,
"mod+shift+f": () => formatQuery,
};

if (!isEmpty(query.query) && canExecuteQuery && !isQueryExecuting) {
extend(shortcuts, {
"mod+enter": doExecuteQuery,
"alt+enter": doExecuteQuery,
});
}

KeyboardShortcuts.bind(shortcuts);
return () => {
KeyboardShortcuts.unbind(shortcuts);
};
}, [
doExecuteQuery,
saveQuery,
openAddNewParameterDialog,
formatQuery,
query.query,
canExecuteQuery,
isQueryExecuting,
]);

const modKey = KeyboardShortcuts.modKey;
}, [canExecuteQuery, isDirty, selectedText, executeQuery, executeAdhocQuery]);

return (
<div className="query-page-wrapper">
Expand Down Expand Up @@ -348,36 +322,30 @@ function QuerySource(props) {

<QueryEditor.Controls
addParameterButtonProps={{
title: (
<React.Fragment>
Add New Parameter (<i>{modKey} + P</i>)
</React.Fragment>
),
title: "Add New Parameter",
shortcut: "mod+p",
onClick: openAddNewParameterDialog,
}}
formatButtonProps={{
title: (
<React.Fragment>
Format Query (<i>{modKey} + Shift + F</i>)
</React.Fragment>
),
title: "Format Query",
shortcut: "mod+shift+f",
onClick: formatQuery,
}}
saveButtonProps={
query.can_edit && {
title: `${modKey} + S`,
text: (
<React.Fragment>
<span className="hidden-xs">Save</span>
{isDirty ? "*" : null}
</React.Fragment>
),
shortcut: "mod+s",
onClick: saveQuery,
}
}
executeButtonProps={{
title: `${modKey} + Enter`,
disabled: isEmpty(query.query) || !canExecuteQuery || isQueryExecuting,
disabled: !canExecuteQuery,
shortcut: "mod+enter, alt+enter",
Copy link
Member

Choose a reason for hiding this comment

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

Very interesting shortcut definition 🚀

Only thing is that for the Query execution button I think it's better if the tooltip shows only "[Mod]+Enter" (even though it also works for Alt) - looks too much with both shortcuts appearing in the tooltip, so perhaps make the tooltip replaceable if you agree?

Copy link
Collaborator Author

@kravets-levko kravets-levko Dec 24, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I decided that it may be useful to see all shortcuts, but I agree with your point - tooltip becomes too crowded. Fixed in 57bf864

onClick: doExecuteQuery,
text: <span className="hidden-xs">{selectedText === null ? "Execute" : "Execute Selected"}</span>,
}}
Expand Down
21 changes: 19 additions & 2 deletions client/app/services/keyboard-shortcuts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,26 @@
import { each, trim, without } from "lodash";
import { each, filter, map, toLower, toString, trim, upperFirst, without } from "lodash";
import Mousetrap from "mousetrap";
import "mousetrap/plugins/global-bind/mousetrap-global-bind";

const modKey = /Mac|iPod|iPhone|iPad/.test(navigator.platform) ? "Cmd" : "Ctrl";

export let KeyboardShortcuts = null; // eslint-disable-line import/no-mutable-exports

export function humanReadableShortcut(shortcut) {
const modifiers = {
mod: upperFirst(modKey),
};

shortcut = toLower(toString(shortcut));
shortcut = filter(map(shortcut.split(","), trim), s => s !== "");
shortcut = map(shortcut, sc => {
sc = filter(map(sc.split("+")), s => s !== "");
return map(sc, s => modifiers[s] || upperFirst(s)).join(" + ");
}).join(", ");

return shortcut !== "" ? shortcut : null;
}

const handlers = {};

function onShortcut(event, shortcut) {
Expand All @@ -13,7 +30,7 @@ function onShortcut(event, shortcut) {
}

function KeyboardShortcutsService() {
this.modKey = /Mac|iPod|iPhone|iPad/.test(navigator.platform) ? "Cmd" : "Ctrl";
this.modKey = modKey;

this.bind = function bind(keymap) {
each(keymap, (fn, key) => {
Expand Down