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

Use Sass 3 (SCSS; Sassy CSS) as a pre-processor for styling #1049

Closed
wants to merge 2 commits into from

Conversation

mtraynham
Copy link
Contributor

Styling is a pain, at least when keeping things consistent. Less & Sass are great for these kinds of things since they map well to the HTML DOM hierarchy rather than the flat list of CSS selectors, also providing variables/mixins.

Sass has a larger community, so that's my reasoning for choosing one over the other.

Ideally, it may even be better to just use d3.js to select these nodes and use SVG attributes instead of styling. CSS (& Sass or Less) is a fairly mute tool here.

@gordonwoodhull
Copy link
Contributor

Looks nice. 20% shorter!

Seems like there should be a way to verify that the new form is equivalent, but I didn't find it in a quick search.

I think it's better to leave anything that is not data-driven in the hands of CSS so that it's easily customizable without writing "real" code. This kind of thing happens otherwise.

@gordonwoodhull gordonwoodhull modified the milestone: v2.0 Feb 9, 2016
@gordonwoodhull
Copy link
Contributor

Okay, I found Alan Hart's Compare CSS Tool and after manually normalizing a bunch of stuff (it doesn't seem to handle ,), I found the following changes, which look harmless but I'll document them here in case we run into trouble later:

image

@gordonwoodhull
Copy link
Contributor

On closer examination, it's 50/50

  • Doesn't look like rect.stack1, rect.stack2 are used (and they look fishy)
  • I'll re-apply Fix for #1046 #1047 dc-chart .axis .grid-line, which came after this PR.
  • empty rules whyyyyy?
  • user-select is still experimental in many browsers, according to MDN, and I was able to repro some ugly selection artifacts on Firefox, Safari, so I brought in this no-select mixin and all is well again.

@gordonwoodhull
Copy link
Contributor

FWIW a couple of selectors were also more specific, like .dc-legend .dc-legend-item instead of .dc-legend-item, and g.dc-legend-item.fadeout - that's usually good.

@gordonwoodhull
Copy link
Contributor

Thanks @mtraynham! I think there is some stuff that could still be simplified, but this helps organize the styles a lot!

Merged for 2.0.0-beta.33

gordonwoodhull added a commit that referenced this pull request Dec 2, 2016
gordonwoodhull added a commit that referenced this pull request Dec 2, 2016
@mtraynham
Copy link
Contributor Author

👍 Thanks @gordonwoodhull !

@mtraynham mtraynham deleted the sass_preprocessing branch December 4, 2016 21:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants