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

[DISCUSS] - Moves the SQL Lab menu before Charts #15915

Closed

Conversation

michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented Jul 27, 2021

SUMMARY

Moves the SQL Lab menu before Charts to make it workflow-oriented. Generally, the most common workflow is:

  1. to connect with database in Data, Persona: Data scientist.
  2. to explore raw data, cleanup and restructure data if needed, prepare data for exploration by creating virtual datasets in SQL Lab, etc, Persona: Data scientist
  3. to explore, slice, and dice data, and to create charts for a dashboard in Explore(Chart); Persona: Data analyst
  4. to view and edit dashboard in Dashboard Persona: Business analyst/business users.

The other possibility is to reorder the objects by usage, and the order would be the opposite. Dashboards -> Charts -> SQL Lab -> Data. This argument may seem counterintuitive to DS, as DS spends the majority of their time handling data. But the number of dashboard consumers is way larger than dashboard creators in large enterprises.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2021-07-27 at 1 53 24 PM

Screen Shot 2021-07-27 at 3 56 43 PM

@junlincc @rusackas

TESTING INSTRUCTIONS

  • Login to Superset
  • Check the main menu

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@codecov
Copy link

codecov bot commented Jul 27, 2021

Codecov Report

Merging #15915 (ca559dc) into master (94e759d) will decrease coverage by 0.14%.
The diff coverage is 55.55%.

❗ Current head ca559dc differs from pull request most recent head 468a857. Consider uploading reports for the commit 468a857 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master   #15915      +/-   ##
==========================================
- Coverage   77.01%   76.86%   -0.15%     
==========================================
  Files         986      986              
  Lines       51978    51985       +7     
  Branches     7091     7090       -1     
==========================================
- Hits        40030    39959      -71     
- Misses      11722    11800      +78     
  Partials      226      226              
Flag Coverage Δ
hive ?
mysql 81.40% <47.05%> (-0.03%) ⬇️
postgres 81.42% <47.05%> (-0.03%) ⬇️
presto 81.16% <47.05%> (-0.02%) ⬇️
python 81.69% <47.05%> (-0.28%) ⬇️
sqlite 81.06% <47.05%> (-0.03%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...l/AdhocFilterEditPopoverSimpleTabContent/index.tsx 65.68% <ø> (ø)
...ponents/controls/VizTypeControl/VizTypeGallery.tsx 93.65% <ø> (ø)
superset/examples/bart_lines.py 27.58% <0.00%> (-0.99%) ⬇️
superset/examples/country_map.py 25.00% <0.00%> (-0.65%) ⬇️
superset/examples/energy.py 26.19% <0.00%> (-0.64%) ⬇️
superset/examples/flights.py 17.64% <0.00%> (-0.54%) ⬇️
superset/examples/long_lat.py 23.80% <0.00%> (-0.59%) ⬇️
superset/examples/multiformat_time_series.py 16.66% <0.00%> (-0.36%) ⬇️
superset/examples/paris.py 26.92% <0.00%> (-1.08%) ⬇️
superset/examples/random_time_series.py 19.44% <0.00%> (-0.56%) ⬇️
... and 11 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94e759d...468a857. Read the comment docs.

@junlincc
Copy link
Member

/testenv up

@github-actions
Copy link
Contributor

@junlincc Ephemeral environment spinning up at http://34.221.101.194:8080. Credentials are admin/admin. Please allow several minutes for bootstrapping and startup.

@john-bodley
Copy link
Member

@michael-s-molina would you be able to document the motivation for the change?

@ktmud
Copy link
Member

ktmud commented Jul 27, 2021

Please make sure PR description has enough context.

Without enough context, it'd just look like "because we talked about this internally so we are doing it", which is very much against the Apache way.

@graceguo-supercat
Copy link

In airbnb, SQL users and Dashboard creation users are overlapped but not same group of people. Many users come to superset only for SQL Lab. This change is minor, but still might make them feel inconvenient and confused. Could you provide more reason for this change? thanks!

@michael-s-molina
Copy link
Member Author

michael-s-molina commented Jul 27, 2021

@john-bodley @ktmud @graceguo-supercat I'll check with @junlincc and @rusackas about the change and provide more context in the PR's description. I'm converting the PR to draft to avoid being merged before the context is provided.

@github-actions
Copy link
Contributor

Ephemeral environment shutdown and build artifacts deleted.

@michael-s-molina michael-s-molina marked this pull request as draft July 27, 2021 20:25
@junlincc
Copy link
Member

Hi @john-bodley
We are trying to clearing some low-hanging fruit that previously discussed with Cartel, and approved in SIP-34.
Menu items in the top nav bar historically were not arranged in any particular order. We re-ordered the nav menu once last year and re-organize items in the sub menu as first step.

In the research with Cartel, the designer suggested that we can either arrange it by user's work flow, which is 1. to connect with database in Data, Persona: Data scientist. -> 2. to explore raw data, cleanup and restructure data if needed, prepare data for exploration by creating virtual datasets in SQL Lab, etc, Persona: Data scientist -> 3. to explore, slice and dice data, and to create charts for dashboard in Explore(Chart); Persona: Data analyst ->4. to view and edit dashboard in Dashboard Persona: Business analyst/business users.
The other idea is to reorder the objects by usage, and the order would be the opposite. Dashboards-> Charts->SQL Lab->Data. This argument may seems counterintuitive to DS, as DS spend majority of their time in handling data. But the number of dashboard consumers is way larger than dashboard creators in large enterprises.

Given the current order, Data -> Charts -> Dashboards -> SQL lab, it seems to be less disruptive if we reorder by workflow.

We also did some user research in-house to validate our assumption, it's not directly related but could use it as a support document.

https://docs.google.com/presentation/d/182UJeK4y-BhcYkWJWYTJSA-O2_2LOXDYoAANi76-uBQ/edit#slide=id.g9743bec153_1_113

@michael-s-molina
Copy link
Member Author

Thanks for your comment @junlincc!

@john-bodley @ktmud @graceguo-supercat I updated the PR's description with Junlin's comments. We are trying to establish the most suited workflow for the majority of organizations using Superset.

Do you agree with the change?

@michael-s-molina michael-s-molina changed the title chore: Moves the SQL Lab menu before Charts [DISCUSS] - Moves the SQL Lab menu before Charts Jul 27, 2021
@ktmud
Copy link
Member

ktmud commented Jul 27, 2021

Personally I think it'd make more sense to order by usage than workflow:

  1. Most users don't need the full workflow: even for Data Scientists, they seldom work through the whole workflow in one go---IMO having the full workflow in the right order is less important than having easiest access to the most used steps in the workflow.
  2. Different users have different workflows: "Data -> SQL Lab -> Chart -> Dashboard" is just one workflow of some Data Scientists. Some more frequent workflows are:
    1. "Chart -> Dashboard"---for Data Scientists who only use pre-configured and shared datasets (which is probably something Superset should encourage), in which case one may even argue the Data tab can be hidden into "Settings" (like Redash does).
    2. "Dashboard -> Chart -> SQL Lab"---for dashboard viewers who are also interested in how the underlying data were computed. This is somewhat similar to Looker's "Browse -> Explore -> Develop".
  3. The workflow order is less important in the main menu: while having the right order is important in some places, it's not like users can or need to return to previous steps by clicking on the main menu when they are creating a dashboard from scratch.
  4. Home page already orders content by usage: it's better be consistent than not.

I went back and checked, SIP-34 actually re-arranged the menu by usage order (Dashboards -> Charts -> SQL Lab -> Data). There must be some reason behind that---hope it was in line with what I described above.

Lastly, even if "not being disruptive" is the only concern, changing from "Data -> Charts -> Dashboards -> SQL Lab" to "Data -> SQL Lab -> Charts -> Dashboards" would change the position of 3 tabs, keeping the least used tab unchanged; while "Dashboards -> Charts -> Data -> SQL Lab" would only change 2, keeping the most used tab unchanged.

@junlincc
Copy link
Member

@ktmud i agree. those routes you mentioned are validated in our research as well.
I personally don't have a strong preference of arranging the order by "Dashboards-> Charts -> SQL lab -> Data" or the other way around, as long as SQL lab is being placed in AN order. I know that this module was added separately and organically before without any design thinking.
Looks like we have reached an agreement here. I'm happy to change in order to move this PR forward :)

@michael-s-molina
Copy link
Member Author

michael-s-molina commented Jul 28, 2021

@junlincc @ktmud @graceguo-supercat @john-bodley @srinify
Thanks for your comments.

Can we proceed and make the change to follow SIP-34 order?
Dashboards -> Charts -> SQL Lab -> Data

@srinify
Copy link
Contributor

srinify commented Jul 28, 2021

Thanks for the discussion y'all, I say let's move forward @michael-s-molina !

@ktmud
Copy link
Member

ktmud commented Jul 28, 2021

I think both "Dashboards -> Charts -> SQL Lab -> Data" and "Dashboards -> Charts -> Data -> SQL Lab" make sense and don't have a strong preference of either.

The former is in line with the order by usage, but although SQL Lab may be more frequently used than Data overall, a lot of users almost never use either. For users do use SQL Lab directly from the main nav, it's a very separate workflow and they tend to use it a lot. So Keeping it as the last item also makes sense.

@junlincc
Copy link
Member

@michael-s-molina please reopen the PR and proceed the change with below order.
Dashboards -> Charts -> SQL Lab -> Data

thanks for everyone's discussion

@michael-s-molina
Copy link
Member Author

@michael-s-molina please reopen the PR and proceed the change with below order.
Dashboards -> Charts -> SQL Lab -> Data

thanks for everyone's discussion

@junlincc I opened a follow-up PR to preserve the original description and screenshots of this PR. Both PRs are referencing each other.

@ktmud
Copy link
Member

ktmud commented Jul 28, 2021

Before we go, I'd like to vent out something that has been brewing in my mind for a while. I've noticed many times that whenever there is a disagreement in UX changes, there is the argument that "we made the choice for the broader Superset community/the majority of Superset users", or the so-called "business users". To me, this argument feels counterproductive.

As shown above, there can be many more substantive arguments about either side of each choice. Even if you made a decision with your version of "the majority of users" in mind, it's possible that there are the other side of the coin or some deal-breaking edge cases not factored into the decision-making process. We raise our concerns based on what our users tell us (and years of experience of dealing with real Superset users), too. It's quite discouraging to having to defend these concerns as if they were only of the interest of a small group (or ourselves).

Let's focus on the substance and make sure all future PRs have logical and thorough reasoning clearly stated BEFORE they are up for review so potential debates can be more efficient. The more well-reasoned and clearly-described a PR description is, the better people can understand your original intention. The better we understand each other, the more efficient the discussion would be.

@junlincc
Copy link
Member

junlincc commented Jul 29, 2021

I agree we should have a better flow for communicating and implementing UI/UX changes, and we will make sure it happens soon.
FYI This change may seem recent, but the actual research that was done to support this change happened last year, 80% of our datapoint is actually collected from airbnb users last year, and if not all, most of the interviewee agreed with changing the order, and the research summary is in
https://docs.google.com/presentation/d/182UJeK4y-BhcYkWJWYTJSA-O2_2LOXDYoAANi76-uBQ/edit
2 Preset designers and I each talked to 6-7 users from Airbnb, connected by @zuzana-vej.

@graceguo-supercat @ktmud @john-bodley @willbarrett @kristw

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

Successfully merging this pull request may close these issues.

6 participants