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 visualizations registry/renderer/editor to React #3493

Merged
merged 52 commits into from
Apr 30, 2019
Merged

Migrate visualizations registry/renderer/editor to React #3493

merged 52 commits into from
Apr 30, 2019

Conversation

kravets-levko
Copy link
Collaborator

@kravets-levko kravets-levko commented Feb 25, 2019

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

  • Refactor

Description

Prepare base for upcoming visualizations migration (one by one).

  • Migrate visualizations registry
  • Migrate Visualization Editor dialog
  • Migrate Visualization Renderer wrapper
  • Update visualizations to work with new infrastructure:
    • Box Plot (deprecated)
    • Chart
    • Choropleth
    • Cohort
    • Counter
    • Funnel
    • Map
    • Pivot
    • Sankey
    • Sunburst
    • Table
    • Word Cloud
  • Cleanup
    • Remove old code
    • Optimize visualizations where possible
      • Map and Choropleth: infinite loop on map bounds sync between editor and preview (fix: proper event handling + update lock)
      • Pivot: optimize updates (don't re-render if only "Hide controls" option changed)
      • Fix deprecated Box Plot (didn't clear container properly on update; didn't properly compute plot width; was accessing all <svg> tags on page 🤦‍♂️)
    • Move grid settings from visualization options to visualization config
  • Tests

Migrated some simplest editors to React (as an example for future refactoring).

Also, this PR required migrating filters to React as well.

Related Tickets & Documents

Replaces #2988

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

Visualizations itself didn't change - they're still rendered by the same AngularJS components. Visualization Editor dialog and Filters are now 100% React/AntD and therefore looks a bit differently.

image

image

image

image

image

image

@ghost ghost assigned kravets-levko Feb 25, 2019
@ghost ghost added the in progress label Feb 25, 2019
@arikfr arikfr added Frontend Frontend: React Frontend codebase migration to React labels Feb 26, 2019
Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

👍

I know this is WIP, but noticed a few things as I was reading that I thought worth pointing out.

client/app/visualizations/table/index.js Show resolved Hide resolved
client/app/visualizations/table/index.js Outdated Show resolved Hide resolved
client/app/visualizations/Visualization.jsx Outdated Show resolved Hide resolved
client/app/visualizations/Visualization.jsx Outdated Show resolved Hide resolved
@kravets-levko kravets-levko marked this pull request as ready for review March 5, 2019 12:47
@kravets-levko kravets-levko changed the title Migrate visualizations registry/renderer/editor to React (WIP) Migrate visualizations registry/renderer/editor to React Mar 5, 2019
@kravets-levko
Copy link
Collaborator Author

@ranbena @arikfr I added some tests - can you review please?

@arikfr
Copy link
Member

arikfr commented Apr 14, 2019

I've noticed we show the Widget title differently now: we only show visualization name without the query name.

Before After
image image

Copy link
Member

@arikfr arikfr left a comment

Choose a reason for hiding this comment

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

Issues I noticed with filters:

  • Previously you could narrow down filter options by typing -- this is no longer possible. I guess it's a simple toggle on the Ant component?
  • We used to show the visualization preview in the editor along with filters -- this is no longer the case.
  • When switching visualizations it resets the filter.

client/app/components/Filters.jsx Show resolved Hide resolved
@arikfr
Copy link
Member

arikfr commented Apr 14, 2019

One more issue with filters:

@kravets-levko
Copy link
Collaborator Author

@arikfr Fixed all notices, can you check it once more please?

@kravets-levko
Copy link
Collaborator Author

@arikfr @ranbena I'm going to merge this, if you have nothing to add

@ranbena
Copy link
Contributor

ranbena commented Apr 30, 2019

@kravets-levko i didn't yet get another chance to look at the newer code but i support merging it anyway. Let's get it out of your way 👍🏻

@kravets-levko kravets-levko merged commit 9a4433b into getredash:master Apr 30, 2019
@arikfr
Copy link
Member

arikfr commented Apr 30, 2019

🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend: React Frontend codebase migration to React Frontend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants