Skip to content

Conversation

@rusackas
Copy link
Member

@rusackas rusackas commented Oct 20, 2020

SUMMARY

Cleaning up various unnecessary classes and such around buttons.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Most should show no significant visible difference. Screenshots of what is visible here:

A little margin between buttons (will PR a better fix for this scenario soon)
before:
image
after:
image

TEST PLAN

Made sure case by case that I didn't break anything. If I did this right, you'll likely never notice.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@codecov-io
Copy link

codecov-io commented Oct 20, 2020

Codecov Report

Merging #11342 into master will decrease coverage by 9.74%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #11342      +/-   ##
==========================================
- Coverage   65.75%   56.01%   -9.75%     
==========================================
  Files         838      406     -432     
  Lines       39714    13562   -26152     
  Branches     3613     3443     -170     
==========================================
- Hits        26115     7597   -18518     
+ Misses      13498     5796    -7702     
- Partials      101      169      +68     
Flag Coverage Δ
#cypress 56.01% <0.00%> (-0.40%) ⬇️
#javascript ?
#python ?

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

Impacted Files Coverage Δ
...end/src/SqlLab/components/ExploreResultsButton.jsx 5.00% <0.00%> (-68.75%) ⬇️
...et-frontend/src/SqlLab/components/LimitControl.tsx 65.78% <ø> (-23.58%) ⬇️
...erset-frontend/src/SqlLab/components/SaveQuery.tsx 61.11% <ø> (-26.99%) ⬇️
...tend/src/SqlLab/components/ScheduleQueryButton.jsx 8.62% <ø> (ø)
...rontend/src/SqlLab/components/ShareSqlLabQuery.jsx 35.48% <ø> (-58.07%) ⬇️
...erset-frontend/src/SqlLab/components/SqlEditor.jsx 49.09% <ø> (-4.25%) ⬇️
...rset-frontend/src/components/ListView/ListView.tsx 75.64% <ø> (-23.28%) ⬇️
...et-frontend/src/components/RefreshChartOverlay.tsx 100.00% <ø> (+11.11%) ⬆️
...end/src/views/CRUD/data/database/DatabaseModal.tsx 2.50% <ø> (-68.93%) ⬇️
...uperset-frontend/src/dashboard/util/dnd-reorder.js 0.00% <0.00%> (-100.00%) ⬇️
... and 662 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 7369039...2235a4d. Read the comment docs.

@rusackas rusackas marked this pull request as ready for review October 21, 2020 05:42
@rusackas rusackas requested a review from mistercrunch October 21, 2020 05:42
@rusackas rusackas merged commit efdda8b into apache:master Oct 23, 2020
@rusackas rusackas deleted the button-cruft-cleanup branch October 23, 2020 17:42
@mistercrunch
Copy link
Member

mistercrunch commented Oct 24, 2020

I think this PR made some <Label /> look funky
Screen Shot 2020-10-23 at 9 49 50 PM
Screen Shot 2020-10-23 at 9 49 45 PM

Actually nevermind it's #11400

@rusackas
Copy link
Member Author

Actually nevermind it's #11400

Ack, then it's still my fault! And I'm still confused! Investigating...

auxten pushed a commit to auxten/incubator-superset that referenced this pull request Nov 20, 2020
* toggleSchedule

* cleaning up run query button

* removing unnecessary (subtle!) style overrides

* bulk action classname to button style prop

* nixing btn-primary

* nixing cancelQuery class

* togglesave

* nixing autocomplete class

* linting

* nixing button classes of 'ok' and 'reset'

* unoing diagnostic test

* begrudgingly adding an m-r-3 class... for now

* this is not a class to worry about

* test fixes for limit control

* updating TODO notes, calling it a day for now.

* gah... copy paste error.
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 First shipped in 1.0.0 labels Mar 12, 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 🚢 1.0.0 First shipped in 1.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants