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

Jump to bottom

Migrate Query pages to React #4429

Merged
merged 35 commits into from
Jan 6, 2020
Merged

Migrate Query pages to React #4429

merged 35 commits into from
Jan 6, 2020

Conversation

gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented Dec 10, 2019 •
edited by kravets-levko
Loading

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

  • Refactor

Description

Main PR for the Query Pages migration.

Status

  • Query View page
  • Query Source page
    • layout (resizable sections)
    • overlays block (permissions and datasource alerts)
    • unsaved changes alert when leaving page
    • left section
      • data source selector
      • schema browser
        • integration with query editor
      • schedule
      • description and metadata
    • top-right section
      • query editor
    • bottom-right section
      • visualizations
      • actions and metadata
    • Refine code
    • Preserve page state when saving new query
  • Common components
    • page header
    • visualization tabs
    • parameters
    • query execution status
  • Desktop and mobile layout
  • Testing
  • Cleanup (PR Migrate Query pages to React: cleanup #4512)

@kravets-levko kravets-levko added Frontend Frontend: React Frontend codebase migration to React labels Dec 10, 2019
@weekly-digest weekly-digest bot mentioned this pull request Dec 16, 2019
Closed
@kravets-levko kravets-levko mentioned this pull request Dec 16, 2019
Merged
1 task
@gabrieldutra gabrieldutra mentioned this pull request Dec 17, 2019
Merged
1 task
@kravets-levko kravets-levko mentioned this pull request Dec 17, 2019
Merged
5 tasks
@gabrieldutra gabrieldutra mentioned this pull request Dec 17, 2019
Merged
1 task
@kravets-levko kravets-levko mentioned this pull request Dec 17, 2019
Merged
1 task
@kravets-levko kravets-levko mentioned this pull request Dec 20, 2019
Merged
1 task
@weekly-digest weekly-digest bot mentioned this pull request Dec 23, 2019
Closed
@kravets-levko kravets-levko mentioned this pull request Dec 23, 2019
Merged
1 task
@arikfr arikfr mentioned this pull request Dec 24, 2019
Closed
@gabrieldutra gabrieldutra mentioned this pull request Dec 29, 2019
Merged
3 tasks
@kravets-levko kravets-levko mentioned this pull request Dec 29, 2019
Merged
1 task
@kravets-levko kravets-levko mentioned this pull request Dec 29, 2019
Merged
1 task
@weekly-digest weekly-digest bot mentioned this pull request Dec 30, 2019
Closed
@kravets-levko kravets-levko mentioned this pull request Dec 30, 2019
Merged
1 task
@kravets-levko kravets-levko mentioned this pull request Dec 30, 2019
Merged
5 tasks
@arikfr
Copy link
Member

arikfr commented Jan 2, 2020

The "Funnel" visualization here: https://deploy-preview-4429--redash-preview.netlify.com/queries/178# breaks rendering (the page disappears). There seems to be an issue with the funnel (it doesn't render on https://redash-preview.netlify.com/queries/178#384 either), but in the previous version it wouldn't disappear.

I guess we should introduce error boundaries around visualizations?

@arikfr
Copy link
Member

arikfr commented Jan 2, 2020

Another weird issue: https://deploy-preview-4429--redash-preview.netlify.com/queries/275?p_user_id=0&p_date_range=d_last_week -> I can't fork this query in Results View, but I can in Query Editor.

@kravets-levko
Copy link
Collaborator

kravets-levko commented Jan 2, 2020 •
edited
Loading

I can't fork this query in Results View, but I can in Query Editor.

Fixed, thank you!

The "Funnel" visualization here: https://deploy-preview-4429--redash-preview.netlify.com/queries/178# breaks rendering (the page disappears). There seems to be an issue with the funnel (it doesn't render on https://redash-preview.netlify.com/queries/178#384 either), but in the previous version it wouldn't disappear.

I guess we should introduce error boundaries around visualizations?

It's not directly related to this PR, I'll open a new one to master (will fix Funnel error and add error boundary to visualization renderer).

Upd.: #4518

@kravets-levko kravets-levko force-pushed the migrate-query-source-view-to-react branch 2 times, most recently from 6e0a214 to 2d2c997 Compare January 2, 2020 15:58
@kravets-levko kravets-levko force-pushed the migrate-query-source-view-to-react branch from 2d2c997 to 2934e53 Compare January 2, 2020 16:25
@weekly-digest weekly-digest bot mentioned this pull request Jan 6, 2020
Closed
@kravets-levko @gabrieldutra
Migrate Query pages to React: cleanup (#4512)
9729b72 
* 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>
@gabrieldutra gabrieldutra marked this pull request as ready for review January 6, 2020 17:48
@kravets-levko kravets-levko merged commit 99c276f into master Jan 6, 2020
@kravets-levko kravets-levko deleted the migrate-query-source-view-to-react branch January 6, 2020 18:51
@kravets-levko
Copy link
Collaborator

kravets-levko commented Jan 6, 2020 •
edited
Loading

Hey @jezdez,

we had a good discussion about adding dynamic components to the query pages, and I promised you to add some. But later I realized that I may not fully understand your needs. Therefore we merged this PR, and now, when master contains updated query pages, it should be easy for you to add dynamic components where you need, and everything will perfectly fit your needs.

Sorry if this is not what you (probably) expected from me, but I hope you see my point. Looking forward to see a PR from you 🙂

@weekly-digest weekly-digest bot mentioned this pull request Jan 13, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jezdez jezdez jezdez left review comments

@kravets-levko kravets-levko kravets-levko left review comments

Assignees
No one assigned
Labels
Frontend: React Frontend codebase migration to React Frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gabrieldutra @arikfr @kravets-levko @jezdez