LESS is more

[rusackas]

CATEGORY

Choose one

• Bug Fix
• Enhancement (new features, refinement)
• Refactor
• Add tests
• Build / Development Environment
• Documentation

SUMMARY

This shouldn't change anything in the processed CSS.

Now that we have LESS everywhere, this PR just goes through and takes advantage of it, tidying up various things readability/grouping

I went through and did an npm run build on the master branch and this feature branch. I compared all resulting CSS files, and screenshots of the (innocuous as far as I can see) diffs are below - it looks like things were just re-ordered a bit:

(this yields the same CSS attributes without repeating the properties)

[codecov-io]

Codecov Report

Merging #9023 into master will increase coverage by 0.29%.
The diff coverage is 80%.

@@            Coverage Diff             @@
##           master    #9023      +/-   ##
==========================================
+ Coverage   59.15%   59.45%   +0.29%     
==========================================
  Files         367      369       +2     
  Lines       11686    11747      +61     
  Branches     2866     2888      +22     
==========================================
+ Hits         6913     6984      +71     
+ Misses       4594     4584      -10     
  Partials      179      179

Impacted Files	Coverage Δ
...assets/src/components/ListView/TableCollection.tsx	92.59% <ø> (ø)	⬆️
...rset/assets/src/components/ConfirmStatusChange.tsx	100% <100%> (ø)
...perset/assets/src/components/ListView/ListView.tsx	92.2% <100%> (+1.03%)	⬆️
superset/assets/src/SqlLab/constants.js	100% <100%> (ø)	⬆️
...et/assets/src/components/IndeterminateCheckbox.jsx	100% <100%> (ø)
superset/assets/src/components/ListView/utils.ts	98.46% <100%> (+0.31%)	⬆️
...uperset/assets/src/SqlLab/components/SqlEditor.jsx	51.94% <100%> (ø)	⬆️
.../assets/src/SqlLab/components/AceEditorWrapper.jsx	54.21% <33.33%> (ø)	⬆️
...t/assets/src/views/dashboardList/DashboardList.tsx	73.39% <61.29%> (+14.44%)	⬆️
... and 7 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 440665c...94cdc14. Read the comment docs.

[craig-rueda]

LGTM, but prob needs to be tested. This definitely makes things easier to read...

[etr2460]

A few comments, but otherwise seems legit. could you add a bunch of before and after screenshots to your test plan to confirm that this works. alternatively, you could attach a diff on the processed CSS to confirm it's all correct

[etr2460]

this is a styling thing, but personally i prefer a new line between normal styles and the &:foo { line. I think there's a way to lint for that, if so maybe you could add that rule in another PR. if not, maybe we could start standardizing that here if you agree?

[rusackas]

I prefer that too, but glad you caught ones I missed. I'll look for a linting rule.

[etr2460]

lol, what was this?

[rusackas]

There are a bunch of fairly convoluted styles around the dashed borders in the dashboard editor, which seem like they could use a simplification effort (a future PR, probably). I'll double check that this one wasn't providing some useful 1px offset.

[rusackas]

You could attach a diff on the processed CSS to confirm it's all correct.

I like this idea... will do later today.

[rusackas]

@etr2460 I pasted a bunch of diff screenshots above. To get these, I ran the build on master, and on this branch. I then took all the resulting CSS files from each build, removed the annoying hashes from the filenames, ran prettier on them so I could get a reasonable DIFF, and then got the DIFF via github. Hopefully I didn't screw anything up in that process, but I'm feeling pretty good about it overall :D

[etr2460]

Thanks for all the diligence in testing and ensuring the outputted CSS is correct. And double thanks for adding the lint rule!