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

chore(sqllab): Change icon color for running sql #22050

Merged

Conversation

justinpark
Copy link
Member

@justinpark justinpark commented Nov 7, 2022

SUMMARY

There's an open issue that is hard to distinguish icons between running and successful query state in sqllab.
This commit adds the animated dimming translation for running statue to be distinguishing the running state easily.
This commit updates the icon color for running state to be distinguishing the running and finish states. It also adds the icons for success and fail status to help red/green colorblind to distinguish the color status.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

  • Before:

Screen_Shot_2022-11-07_at_2_49_13_PM

  • After:

Screen Shot 2022-11-08 at 11 09 38 AM

Grey: Not yet started
Blue: In progress*
Green: Success
Red: Failed
Note people are red/green colorblind so maybe having a check-mark/x-mark inside the circle would help to distinguish between (3) and (4).

TESTING INSTRUCTIONS

Go to sqllab
Run a query

ADDITIONAL INFORMATION

  • Has associated issue: issues#11123 issues#13648 discussion#18402
  • Required feature flags:
  • 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 Nov 8, 2022

Codecov Report

Merging #22050 (b8be2b7) into master (06f87e1) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##           master   #22050   +/-   ##
=======================================
  Coverage   66.97%   66.97%           
=======================================
  Files        1832     1832           
  Lines       69905    69909    +4     
  Branches     7570     7571    +1     
=======================================
+ Hits        46820    46823    +3     
  Misses      21127    21127           
- Partials     1958     1959    +1     
Flag Coverage Δ
javascript 53.77% <80.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
...tend/packages/superset-ui-core/src/style/index.tsx 100.00% <ø> (ø)
...tend/src/SqlLab/components/TabStatusIcon/index.tsx 80.00% <80.00%> (-20.00%) ⬇️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@john-bodley
Copy link
Member

john-bodley commented Nov 8, 2022

Woot! I never realized that the icon changed. @justinpark rather than flashing I was wondering whether a different color would be more intuitive, i.e.,

  1. Grey: Not yet started
  2. Blue: In progress*
  3. Green: Success
  4. Red: Failed

Note people are red/green colorblind so maybe having a check-mark/x-mark inside the circle would help to distinguish between (3) and (4).

* I think blue is likely better than orange as orange normally implies warning.

@justinpark
Copy link
Member Author

Woot! I never realized that the icon changed. justinpark rather than flashing I was wondering whether a different color would be more intuitive, i.e.,

  1. Grey: Not yet started
  2. Blue: In progress*
  3. Green: Success
  4. Red: Failed

Note people are red/green colorblind so maybe having a check-mark/x-mark inside the circle would help to distinguish between (3) and (4).

  • I think blue is likely better than orange as orange normally implies warning.

@john-bodley that's good idea. I updated the icons accordingly.

@justinpark justinpark changed the title chore(sqllab): Dimming status icon for running sql chore(sqllab): Change icon color for running sql Nov 8, 2022
@justinpark
Copy link
Member Author

cc: @michael-s-molina @eschutho

@eschutho
Copy link
Member

This is awesome @justinpark!

@justinpark justinpark force-pushed the chore--dimming-status-color-for-running-query branch from 9ac566a to b8be2b7 Compare November 15, 2022 18:26
@ktmud
Copy link
Member

ktmud commented Nov 15, 2022

Nice

@ktmud ktmud merged commit 4f2e264 into apache:master Nov 15, 2022
justinpark added a commit to airbnb/superset-fork that referenced this pull request Nov 30, 2022
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 2.1.0 and removed 🚢 2.1.3 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/M 🚢 2.1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants