Migrate Funnel visualization to React

[kravets-levko]

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

• Refactor
• Feature

Description

• Convert component to React
  • Renderer
  • Editor
• Refine and performance
  • Get rid of "Auto sort" option: replace with "Sort column" dropdown + "Sort reverse" switch
  • Add option for number format (currently hard-coded)
  • Add option to limit items count (currently hard-coded to 100)
  • Add options for percent values bounds (currently hardcoded to 0.01%..1000%)
  • Debounce inputs
• Cleanup
  • Get rid of d3 dependency
• Tests

Related Tickets & Documents

#3301 (Migrate Visualizations to React -> Funnel)

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

[kravets-levko]

Everything as usual - I need to add some tests, but the rest is ready for review 🙂

[arikfr]

• Get rid of "Auto sort" option: replace with "Sort column" dropdown + "Sort reverse" switch

Can you explain what you mean here?

[kravets-levko]

There was "Auto sort" option: when it was true, data was sorted by value descending, when false - user was able to specify a column that was used to sort data ascending only. I replaced this option with two: sort column and asc/desc switch (with code that maps old option to new ones). To achieve autoSort = true, user should sort data by column used for value, and choose reverse order. autoSort = false actually works as previously - user need to select a column for sorting, but now they can also select a direction.

[ranbena]

A bug I found:

1. Edit viz for https://deploy-preview-4267--redash-preview.netlify.com/queries/47#111
2. Change Step Column Name to "count"
3. Change back to "name"

Expected - all column names return to "name"
Actual - some column names remain "count"

(I presume this has to do with non-uniqueness of count values)

[kravets-levko]

@ranbena Column name options are not connected - you can choose them independently. So that's okay if you change one and others remains unchanged. But when trying to reproduce your bug, I found another one (in renderer), so anyway there is something to fix 🙂

[ranbena]

👌

[arikfr]

Are we sure people would ever want to sort funnel values in an ascending order?

If we decide to keep the extra option (sort order), we need to make sure that the current UX is kept, which is: when you pick the values column you get a sorted result by the values column in a descending order. At the moment, after picking the values column you need to manually pick sort column and change the the order.

[kravets-levko]

@arikfr While I think it's not a problem to change two more fields (Funnel does not have so much options as, say, Chart), but I got your point. What about compromise: when user select a value column, and sort column is not set or is the same as value column - change sort column as well and set reverse order. WDYT?

[arikfr]

While I think it's not a problem to change two more fields (Funnel does not have so much options as, say, Chart)

We should strive for things to become simpler and not to complicated everything equally 😆

If 99% of the time people want some behavior we better provide it as the default.

What about compromise: when user select a value column, and sort column is not set or is the same as value column - change sort column as well and set reverse order. WDYT?

Yes this can work.

We need to change the name of the "Reverse order" setting. Because for humans, in the context of funnels, the descending order is the right order. We can change it "Descending Sort" and it will be on by default.

[kravets-levko]

Okay. Then I'll experiment with it later and will ping you 👍

[kravets-levko]

@arikfr I decided to bring back "Auto Sort" option, but renamed it to "Custom Sorting" (see screenshots/check preview), and added sort direction dropdown (wasn't available prior to this PR):

[arikfr]

👍