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: refine code #4499

Merged
merged 9 commits into from
Dec 27, 2019
Merged

Migrate Query Source page to React: refine code #4499

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
f2a79f5
Refine Query Source: extract query flags, data sources and schema to …
kravets-levko Dec 27, 2019
e63a66f
Move hooks to own folder
kravets-levko Dec 27, 2019
da00d87
Extract dialogs to hooks
kravets-levko Dec 27, 2019
5a22fbf
Extract more hooks: query, parameters, autocomplete flags, more dialo…
kravets-levko Dec 27, 2019
a49aa20
Query Source page: extract formatQuery function, simplify event handlers
kravets-levko Dec 27, 2019
9d5588b
More accurate hooks dependencies
kravets-levko Dec 27, 2019
844d10b
Convert actions to hooks (most of them); wrap permissions and API key…
kravets-levko Dec 27, 2019
b200465
Convert query update actions to hooks
kravets-levko Dec 27, 2019
f87b116
Refine code
kravets-levko Dec 27, 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
4 changes: 2 additions & 2 deletions client/app/components/queries/ApiKeyDialog/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { clone, extend } from "lodash";
import { extend } from "lodash";
import React, { useMemo, useState, useCallback } from "react";
import PropTypes from "prop-types";
import Modal from "antd/lib/modal";
Expand All @@ -22,7 +22,7 @@ function ApiKeyDialog({ dialog, ...props }) {
.post(`api/queries/${query.id}/regenerate_api_key`)
.success(data => {
setUpdatingApiKey(false);
setQuery(extend(clone(query), { api_key: data.api_key }));
setQuery(extend(query.clone(), { api_key: data.api_key }));
})
.error(() => {
setUpdatingApiKey(false);
Expand Down
40 changes: 20 additions & 20 deletions client/app/components/queries/QueryEditor/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useEffect, useMemo, useRef, useState, useCallback, useImperativeHandle } from "react";
import React, { useEffect, useMemo, useState, useCallback, useImperativeHandle } from "react";
import PropTypes from "prop-types";
import cx from "classnames";
import { AceEditor, snippetsModule, updateSchemaCompleter } from "./ace";
Expand All @@ -15,7 +15,7 @@ const QueryEditor = React.forwardRef(function(
ref
) {
const [container, setContainer] = useState(null);
const editorRef = useRef(null);
const [editorRef, setEditorRef] = useState(null);
Copy link
Member

Choose a reason for hiding this comment

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

Why change it to state?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It was needed to implemented cleanup: useEffect does not see changes when ref value changes (e.g. when someRef.current changes - component will not re-render).


// For some reason, value for AceEditor should be managed in this way - otherwise it goes berserk when selecting text
const [currentValue, setCurrentValue] = useState(value);
Expand Down Expand Up @@ -44,17 +44,19 @@ const QueryEditor = React.forwardRef(function(
);

useEffect(() => {
if (editorRef.current) {
const { editor } = editorRef.current;
updateSchemaCompleter(editor.id, schema); // TODO: cleanup?
if (editorRef) {
const editorId = editorRef.editor.id;
updateSchemaCompleter(editorId, schema);
return () => {
updateSchemaCompleter(editorId, null);
};
}
}, [schema]);
}, [schema, editorRef]);

useEffect(() => {
function resize() {
if (editorRef.current) {
const { editor } = editorRef.current;
editor.resize();
if (editorRef) {
editorRef.editor.resize();
}
}

Expand All @@ -63,16 +65,15 @@ const QueryEditor = React.forwardRef(function(
const unwatch = resizeObserver(container, resize);
return unwatch;
}
}, [container]);
}, [container, editorRef]);

const handleSelectionChange = useCallback(
selection => {
const { editor } = editorRef.current;
const rawSelectedQueryText = editor.session.doc.getTextRange(selection.getRange());
const rawSelectedQueryText = editorRef.editor.session.doc.getTextRange(selection.getRange());
const selectedQueryText = rawSelectedQueryText.length > 1 ? rawSelectedQueryText : null;
onSelectionChange(selectedQueryText);
},
[onSelectionChange]
[editorRef, onSelectionChange]
);

const initEditor = useCallback(editor => {
Expand Down Expand Up @@ -113,28 +114,27 @@ const QueryEditor = React.forwardRef(function(
ref,
() => ({
paste: text => {
if (editorRef.current) {
const { editor } = editorRef.current;
if (editorRef) {
const { editor } = editorRef;
editor.session.doc.replace(editor.selection.getRange(), text);
const range = editor.selection.getRange();
onChange(editor.session.getValue());
editor.selection.setRange(range);
}
},
focus: () => {
if (editorRef.current) {
const { editor } = editorRef.current;
editor.focus();
if (editorRef) {
editorRef.editor.focus();
}
},
}),
[onChange]
[editorRef, onChange]
);

return (
<div className={cx("query-editor-container", className)} {...props} ref={setContainer}>
<AceEditor
ref={editorRef}
ref={setEditorRef}
theme="textmate"
mode={syntax || "sql"}
value={currentValue}
Expand Down
Loading