Skip to content

Conversation

@rlueder
Copy link

@rlueder rlueder commented Nov 30, 2016

What's in this PR?

This is a big refactor of _colors.scss, looking forward to feedback on the changes.

Main goals were to make the stylesheet easier to scan and avoiding a few semantics issues e.g.

.task-new-form-body {
border-color: $idea-background;
}

Hopefully it’ll also help newcomers avoid guesswork when having to assign colors to elements.

References

Progress on: #672

Rafael Lüder added 2 commits November 29, 2016 18:56
Still work in progress, we have quite a few gray colors that could use
some help normalizing. Also a few issues with semantics where the color
variable used is not very intuitive (e.g. border-color:
$idea-background )
@rlueder
Copy link
Author

rlueder commented Nov 30, 2016

I noticed I left a few classes behind under the Buttons and Borders section, I'll update those soon.

I also don't want to alter the design too much but I'd like to have a little more controlled variation of shades of color (grays especially, we had close to 10 variations). I'll work on being consistent with the original design by using sass color functions like darken and lighten

@rlueder
Copy link
Author

rlueder commented Nov 30, 2016

A little more context on the approach I took with the refactoring, the comments are interesting (until one guy hijacks them): https://www.bensmithett.com/stop-using-so-many-sass-variables/

Copy link
Contributor

@begedin begedin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Really, the biggest gain here is the guideline text 😄

@joshsmith Feel free to take a look as well and approve if you feel it's good to go. We do have a couple of css-related PRs up, though, so we should make sure none of them conflict.

@rlueder
Copy link
Author

rlueder commented Dec 1, 2016

Doing a bit more research on making the most of Sass functions for colors, another good resource:
Aesthetic Sass 2: Colors and Palettes

@joshsmith
Copy link
Contributor

I still need to look at this but may not be until beginning of next week.

@rlueder can you help to ensure your changes don't get unmanageable for you by rebasing onto upstream develop regularly and fixing merge conflicts?

Rafael Lüder added 6 commits December 2, 2016 19:01
This commit is mainly experimental. I removed superfluous div elements
from the main header and footer, rearranged some of the folders under
/app/styles and added shame.css for temporary styles we want refactored
or removed in the future. All but one test is passing (related to
ESLint) which is a good thing.
Large refactoring of header, footer and content areas. Also refactored
Sign In, Sign Up and onboarding flow (hello, skills, expertise,
interests).

TO-DO: image-drop needs component and user dropdown need styling
NOTE: to test add the class .site__content--starter to pages that are
part of the onboarding flow (footer of these pages is not separate from
content block and needs a few different flexbox settings, perhaps it'd
be easier to treat it as a footer? WIP)
@rlueder rlueder closed this Dec 8, 2016
@rlueder rlueder deleted the 672_BEM-refactor-colors.scss branch December 8, 2016 16:54
@rlueder
Copy link
Author

rlueder commented Dec 8, 2016

Closed in favor of #846 which is way more comprehensive than this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants