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

Cleanup modules Sass #4342

Merged
merged 3 commits into from
Oct 30, 2014
Merged

Cleanup modules Sass #4342

merged 3 commits into from
Oct 30, 2014

Conversation

PaulAdamDavis
Copy link
Member

References #4107

This is the first step in a larger project to reformat our Sass. Each 'chunk' will reformat a directory in core/client/assets/sass.

The main changes here are:

  • Rename helpers -> modules
  • Rename lib -> vendor (a common practice from JS dev)
  • DRY up icon styles by using a placeholder selector for base icon styles
  • Add new comment block at the tops of files explaining what's what and list the main parts of the file
  • Removes unused and unnecessary mixins & variables
  • Improve var names and add a couple new ones

@PaulAdamDavis
Copy link
Member Author

Looking for feedback & discussion on this. It'll help shape how we do Sass/CSS in the future.

$rounded: 3px;
$shadow: rgba(0,0,0,0.05) 0 1px 5px;
$default-transition-duration: 0.3s;
$side-outlet-transition-duration: 0.4s;

This comment was marked as abuse.

This comment was marked as abuse.

@PaulAdamDavis PaulAdamDavis changed the title [WIP] Sass cleanup [WIP] Cleanup modules Sass Oct 30, 2014
@JohnONolan
Copy link
Member

Looks good. I think some of these commits can be combined though. Commits 2, 3, 4, 6, and 8 can all be squashed into "var cleanup". 1 and 7 can be squashed into file structure cleanup. 5 can live on its own

Also introduces a new format of file opening comment, to outline what happens and a contents list.
@PaulAdamDavis PaulAdamDavis changed the title [WIP] Cleanup modules Sass Cleanup modules Sass Oct 30, 2014
@PaulAdamDavis
Copy link
Member Author

Pending tests, this is now GTG.

JohnONolan added a commit that referenced this pull request Oct 30, 2014
@JohnONolan JohnONolan merged commit 21bbb62 into TryGhost:master Oct 30, 2014
@JohnONolan
Copy link
Member

👍

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.

None yet

2 participants