Migrate Query Source page to React: Editor area #4468

kravets-levko · 2019-12-20T14:27:35Z

What type of PR is this? (check all applicable)

Refactor

Related Tickets & Documents

#4429

gabrieldutra

Gave up on having a separate QueryEditorComponent? 🙂

client/app/components/queries/QueryEditor/index.jsx

kravets-levko · 2019-12-20T18:28:49Z

Gave up on having a separate QueryEditorComponent?

Not really 😄 I want to get the entire page working, and then I'll have better vision on the entire thing, and will start decomposing it into components/hooks/etc.

kravets-levko · 2019-12-20T18:32:57Z

@gabrieldutra I think tomorrow I'll use your execute query hook to make Execute button working as well, so probably will request review once more

…source-to-react--editor

kravets-levko · 2019-12-23T11:17:34Z

@gabrieldutra I implemented "Execute" button and added execution status block - please review it once more. Thanks!

gabrieldutra

Another note: the Ctrl/Cmd + Enter is not triggering query execution (don't know if you wanted to do it here, but it makes sense).

gabrieldutra · 2019-12-23T17:07:45Z

client/app/pages/queries/components/QueryExecutionStatus.jsx

+  }
+
+  return (
+    <Alert


I think using banner styling may result in a better-looking alert, but perhaps it's worth it to change Antd Alert style globally (not in here of course).

client/app/components/queries/QueryEditor/index.jsx

client/app/pages/queries/QuerySource.jsx

gabrieldutra

Found two issues:
1 - We should check canExecuteQuery for query execution through key press - otherwise it's possible to bypass it (it also creates issues with Parameters Apply All). Perhaps only bind it when canExecuteQuery is true?
2 - Turns out it's also necessary to update dirtyParameters when [parameters] changes, in the query view you couldn't change Parameters, so I didn't know about it before 😅. In any case, you can reproduce a bug when you have Parametes to be applied and you delete the query text containing that parameter.

client/app/pages/queries/QuerySource.jsx

gabrieldutra · 2019-12-24T02:31:44Z

client/app/pages/queries/QuerySource.jsx

-                      title: `${modKey} + Enter`,
-                      disabled: isEmpty(query.query) || !canExecuteQuery || isQueryExecuting,
+                      disabled: !canExecuteQuery,
+                      shortcut: "mod+enter, alt+enter",


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?

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

* Migrate Query Source View page to React: skeleton * Sync QueryView and QuerySource (#4430) * Migrate schema browser to react (#4432) * Restyle code with Prettier * Migrate Query page to React: Save changes (#4452) * Migrate query source to React: Set of updates (#4457) * Migrate Query page to React: Visualization Tabs (#4453) Co-Authored-By: Levko Kravets <levko.ne@gmail.com> * Migrate Query Source page to React: Visualizations area (#4463) * Migrate Query page to React: Delete visualization button (#4461) * Migrate Query Source page to React: Visualization actions (#4467) * Migrate Query pages to React: Execute query hook (#4470) * Migrate Query Source page to React: Editor area (#4468) * Migrate Query Source page to React: metadata, schedule and description blocks (#4476) * Migrate Query page to React: Cancel query execution (#4496) * Migrate Query Source page to React: refine code (#4499) * Migrate Query Source page to React: alerts (#4504) * Migrate Query Source page to React: unsaved changes alert (#4505) * Migrate Query Source to React: resizable areas (v2) (#4503) * Migrate Query page to React: Query View (#4455) Co-authored-by: Levko Kravets <levko.ne@gmail.com> * Switch React and Angular versions of pages (until Angular version removed) * Migrate Query pages to React: fix permissions (#4506) * Migrate Query Source page to React: don't reload when saving new query (#4507) * Migrate Query pages to React: fix tests (#4509) * Use skipParametersDirtyFlag in executeQuery * Fix: cannot fork query from Query View page * Optimize query editor: handle query text changes faster * Revert "Optimize query editor: handle query text changes faster" This reverts commit 2934e53. * Reduce debounced time to 100 * Migrate Query pages to React: cleanup (#4512) * Migrate Query pages to React: cleanup * Further cleanup * Remove unused dependencies * Fix embed pages * Attempt to fix flaky test * Cleanup: explicitly register the last Angular component * Move contents of /filters folder to /lib * Remove unnecessary import * Remove cy.wait from Parameters spec Co-authored-by: Gabriel Dutra <nesk.frz@gmail.com> Co-authored-by: Levko Kravets <levko.ne@gmail.com>

Migrate Query Source page to React: Query Editor section (top-right)

e53e10c

kravets-levko added Frontend Frontend: React Frontend codebase migration to React labels Dec 20, 2019

kravets-levko requested a review from gabrieldutra December 20, 2019 14:27

kravets-levko self-assigned this Dec 20, 2019

kravets-levko changed the base branch from master to migrate-query-source-view-to-react December 20, 2019 14:27

kravets-levko added 2 commits December 20, 2019 16:39

Integration with SchemaBrowser

2cdefa2

Add Parameter dialog + update parameters on query source change

86821a6

kravets-levko marked this pull request as ready for review December 20, 2019 15:16

kravets-levko added 2 commits December 20, 2019 17:18

Code style

686c498

Properly clone query object on updates

653dd3f

kravets-levko changed the title ~~Migrate Query Source page to React: Editor section~~ Migrate Query Source page to React: Editor area Dec 20, 2019

kravets-levko mentioned this pull request Dec 20, 2019

Migrate Query Source page to React: Visualization actions #4467

Merged

1 task

gabrieldutra reviewed Dec 20, 2019

View reviewed changes

client/app/components/queries/QueryEditor/index.jsx Show resolved Hide resolved

kravets-levko changed the title ~~Migrate Query Source page to React: Editor area~~ (WIP) Migrate Query Source page to React: Editor area Dec 20, 2019

gabrieldutra approved these changes Dec 20, 2019

View reviewed changes

Merge branch 'migrate-query-source-view-to-react' into migrate-query-…

d4a8426

…source-to-react--editor

weekly-digest bot mentioned this pull request Dec 23, 2019

Weekly Digest (16 December, 2019 - 23 December, 2019) #4474

Closed

restyled-io bot mentioned this pull request Dec 23, 2019

Restyle (WIP) Migrate Query Source page to React: Editor area #4475

Closed

Implement Execute query button

126eeba

kravets-levko force-pushed the migrate-query-source-to-react--editor branch from 7571a17 to 126eeba Compare December 23, 2019 11:14

kravets-levko requested a review from gabrieldutra December 23, 2019 11:15

kravets-levko changed the title ~~(WIP) Migrate Query Source page to React: Editor area~~ Migrate Query Source page to React: Editor area Dec 23, 2019

kravets-levko added 2 commits December 23, 2019 17:56

Fix crash when query (new) has no visualizations yet

f8631c1

Fix Execute query bugs

14e0b36

gabrieldutra reviewed Dec 23, 2019

View reviewed changes

restyled-io bot mentioned this pull request Dec 23, 2019

Restyle Migrate Query Source page to React: Editor area #4478

Closed

Keyboard shortcuts; handle parameters changes

d857074

kravets-levko force-pushed the migrate-query-source-to-react--editor branch from 2320544 to d857074 Compare December 23, 2019 18:42

Improve useQueryExecute hook

1157bbb

gabrieldutra reviewed Dec 23, 2019

View reviewed changes

client/app/pages/queries/QuerySource.jsx Outdated Show resolved Hide resolved

gabrieldutra and others added 4 commits December 23, 2019 17:09

Only bind execute keys when possible to execute

63c24f9

Update dirtyParameters with parameter changes

afcd73d

Move recordEvent to useQueryExecute

8cf3e39

Refine shortcuts binding

16057bf

kravets-levko force-pushed the migrate-query-source-to-react--editor branch from e4e50db to 16057bf Compare December 23, 2019 22:04

gabrieldutra reviewed Dec 24, 2019

View reviewed changes

Show only primary shortcut

57bf864

gabrieldutra approved these changes Dec 24, 2019

View reviewed changes

kravets-levko merged commit cad0c90 into migrate-query-source-view-to-react Dec 24, 2019

kravets-levko deleted the migrate-query-source-to-react--editor branch December 24, 2019 12:40

weekly-digest bot mentioned this pull request Dec 30, 2019

Weekly Digest (23 December, 2019 - 30 December, 2019) #4508

Closed

weekly-digest bot mentioned this pull request Jan 13, 2020

Weekly Digest (6 January, 2020 - 13 January, 2020) #4539

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate Query Source page to React: Editor area #4468

Migrate Query Source page to React: Editor area #4468

kravets-levko commented Dec 20, 2019 •

edited

Loading

gabrieldutra left a comment

kravets-levko commented Dec 20, 2019

kravets-levko commented Dec 20, 2019

kravets-levko commented Dec 23, 2019

gabrieldutra left a comment

gabrieldutra Dec 23, 2019 •

edited

Loading

gabrieldutra left a comment

gabrieldutra Dec 24, 2019

kravets-levko Dec 24, 2019 •

edited

Loading

Migrate Query Source page to React: Editor area #4468

Migrate Query Source page to React: Editor area #4468

Conversation

kravets-levko commented Dec 20, 2019 • edited Loading

What type of PR is this? (check all applicable)

Related Tickets & Documents

gabrieldutra left a comment

Choose a reason for hiding this comment

kravets-levko commented Dec 20, 2019

kravets-levko commented Dec 20, 2019

kravets-levko commented Dec 23, 2019

gabrieldutra left a comment

Choose a reason for hiding this comment

gabrieldutra Dec 23, 2019 • edited Loading

Choose a reason for hiding this comment

gabrieldutra left a comment

Choose a reason for hiding this comment

gabrieldutra Dec 24, 2019

Choose a reason for hiding this comment

kravets-levko Dec 24, 2019 • edited Loading

Choose a reason for hiding this comment

kravets-levko commented Dec 20, 2019 •

edited

Loading

gabrieldutra Dec 23, 2019 •

edited

Loading

kravets-levko Dec 24, 2019 •

edited

Loading