Add Sass mixins and use them. #2904

Merged
merged 2 commits into from Nov 20, 2014

Conversation

Projects
None yet
7 participants
@XhmikosR
Contributor

XhmikosR commented Sep 10, 2014

Reduces code duplication and makes things cleaner.

@XhmikosR XhmikosR changed the title from Add SASS mixins and use them. to Add Sass mixins and use them. Sep 10, 2014

@kleinfreund

This comment has been minimized.

Show comment
Hide comment
@kleinfreund

kleinfreund Sep 11, 2014

Contributor

This makes it easier to edit the Sass files, but doesn't actually reduce code duplication. Basically it's autoprefixing by hand.

Contributor

kleinfreund commented Sep 11, 2014

This makes it easier to edit the Sass files, but doesn't actually reduce code duplication. Basically it's autoprefixing by hand.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 11, 2014

Contributor

It is reducing the code to half even if it's only for some vendor prefixes.
On Sep 11, 2014 10:41 AM, "Philipp Rudloff" notifications@github.com
wrote:

This makes it easier to edit the Sass files, but doesn't actually reduce
code duplication. Basically it's autoprefixing by hand.


Reply to this email directly or view it on GitHub
#2904 (comment).

Contributor

XhmikosR commented Sep 11, 2014

It is reducing the code to half even if it's only for some vendor prefixes.
On Sep 11, 2014 10:41 AM, "Philipp Rudloff" notifications@github.com
wrote:

This makes it easier to edit the Sass files, but doesn't actually reduce
code duplication. Basically it's autoprefixing by hand.


Reply to this email directly or view it on GitHub
#2904 (comment).

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Sep 11, 2014

Contributor

Plus everyone is welcome to extend this. I simply made the start.
On Sep 11, 2014 10:44 AM, "XhmikosR" xhmikosr@gmail.com wrote:

It is reducing the code to half even if it's only for some vendor prefixes.
On Sep 11, 2014 10:41 AM, "Philipp Rudloff" notifications@github.com
wrote:

This makes it easier to edit the Sass files, but doesn't actually reduce
code duplication. Basically it's autoprefixing by hand.


Reply to this email directly or view it on GitHub
#2904 (comment).

Contributor

XhmikosR commented Sep 11, 2014

Plus everyone is welcome to extend this. I simply made the start.
On Sep 11, 2014 10:44 AM, "XhmikosR" xhmikosr@gmail.com wrote:

It is reducing the code to half even if it's only for some vendor prefixes.
On Sep 11, 2014 10:41 AM, "Philipp Rudloff" notifications@github.com
wrote:

This makes it easier to edit the Sass files, but doesn't actually reduce
code duplication. Basically it's autoprefixing by hand.


Reply to this email directly or view it on GitHub
#2904 (comment).

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 5, 2014

Member

Maybe these should be added to the master file? I feel :( about adding yet another file.

Member

parkr commented Oct 5, 2014

Maybe these should be added to the master file? I feel :( about adding yet another file.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Oct 5, 2014

Contributor

@parkr: what master file?

Contributor

XhmikosR commented Oct 5, 2014

@parkr: what master file?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Oct 5, 2014

Member

@parkr: what master file?

site/css/screen.scss I think?

Member

parkr commented Oct 5, 2014

@parkr: what master file?

site/css/screen.scss I think?

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Oct 5, 2014

Contributor

But the whole idea is to include the mixins from other files.
On Oct 5, 2014 8:51 PM, "Parker Moore" notifications@github.com wrote:

@parkr https://github.com/parkr: what master file?

site/css/screen.scss I think?


Reply to this email directly or view it on GitHub
#2904 (comment).

Contributor

XhmikosR commented Oct 5, 2014

But the whole idea is to include the mixins from other files.
On Oct 5, 2014 8:51 PM, "Parker Moore" notifications@github.com wrote:

@parkr https://github.com/parkr: what master file?

site/css/screen.scss I think?


Reply to this email directly or view it on GitHub
#2904 (comment).

@albertogg

This comment has been minimized.

Show comment
Hide comment
@albertogg

albertogg Oct 6, 2014

Member

I think this is useful and having it in a separate file like _mixins.scss is the common way to have it or at least is what I see in other projects 😅. I'm in favor of this 👍

Member

albertogg commented Oct 6, 2014

I think this is useful and having it in a separate file like _mixins.scss is the common way to have it or at least is what I see in other projects 😅. I'm in favor of this 👍

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Oct 26, 2014

Contributor

Note to self: move .header-link and the relevant classes in style.scss. Also change the includes order to have style.scss last for precedence reasons.

Contributor

XhmikosR commented Oct 26, 2014

Note to self: move .header-link and the relevant classes in style.scss. Also change the includes order to have style.scss last for precedence reasons.

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Oct 27, 2014

Contributor

Updated.

Contributor

XhmikosR commented Oct 27, 2014

Updated.

XhmikosR added some commits Sep 10, 2014

Add Sass mixins and use them.
Reduces code duplication and makes things cleaner.
Move custom code from _font-awesome.scss to _style.scss.
Also change the order of includes so that _style.scss is last for precedence reasons.
@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Nov 18, 2014

Contributor

Bump.

Contributor

XhmikosR commented Nov 18, 2014

Bump.

@parkr

This comment has been minimized.

Show comment
Hide comment
Member

parkr commented Nov 18, 2014

/cc @jglovier

@troyswanson

This comment has been minimized.

Show comment
Hide comment
@troyswanson

troyswanson Nov 18, 2014

Member

I was going to mention maybe doing this in #3123, but this work is already done! Looks great 👍

Member

troyswanson commented Nov 18, 2014

I was going to mention maybe doing this in #3123, but this work is already done! Looks great 👍

@jglovier

This comment has been minimized.

Show comment
Hide comment
@jglovier

jglovier Nov 19, 2014

Member

Excellent!! I probably should have done this in #3123 TBH, so 🍻 for picking up the ball. 👍

clap-transformers-bumblebee

Member

jglovier commented Nov 19, 2014

Excellent!! I probably should have done this in #3123 TBH, so 🍻 for picking up the ball. 👍

clap-transformers-bumblebee

@XhmikosR

This comment has been minimized.

Show comment
Hide comment
@XhmikosR

XhmikosR Nov 19, 2014

Contributor

Actually, I've made this PR since September 10...

Contributor

XhmikosR commented Nov 19, 2014

Actually, I've made this PR since September 10...

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Nov 20, 2014

Member

Any worries about accessibility for newbies? If this is what we think newbies ought to see, then 👍

Member

parkr commented Nov 20, 2014

Any worries about accessibility for newbies? If this is what we think newbies ought to see, then 👍

@troyswanson

This comment has been minimized.

Show comment
Hide comment
@troyswanson

troyswanson Nov 20, 2014

Member

No, sir, this is some proper Sass.

On Wed, Nov 19, 2014, 8:59 PM Parker Moore notifications@github.com wrote:

Any worries about accessibility for newbies? If this is what we think
newbies ought to see, then [image: 👍]


Reply to this email directly or view it on GitHub
#2904 (comment).

Member

troyswanson commented Nov 20, 2014

No, sir, this is some proper Sass.

On Wed, Nov 19, 2014, 8:59 PM Parker Moore notifications@github.com wrote:

Any worries about accessibility for newbies? If this is what we think
newbies ought to see, then [image: 👍]


Reply to this email directly or view it on GitHub
#2904 (comment).

@albertogg

This comment has been minimized.

Show comment
Hide comment
@albertogg

albertogg Nov 20, 2014

Member

👍


Alberto Grespan

On Wed, Nov 19, 2014 at 10:53 PM, Troy Swanson notifications@github.com
wrote:

No, sir, this is some proper Sass.
On Wed, Nov 19, 2014, 8:59 PM Parker Moore notifications@github.com wrote:

Any worries about accessibility for newbies? If this is what we think
newbies ought to see, then [image: 👍]


Reply to this email directly or view it on GitHub
#2904 (comment).


Reply to this email directly or view it on GitHub:
#2904 (comment)

Member

albertogg commented Nov 20, 2014

👍


Alberto Grespan

On Wed, Nov 19, 2014 at 10:53 PM, Troy Swanson notifications@github.com
wrote:

No, sir, this is some proper Sass.
On Wed, Nov 19, 2014, 8:59 PM Parker Moore notifications@github.com wrote:

Any worries about accessibility for newbies? If this is what we think
newbies ought to see, then [image: 👍]


Reply to this email directly or view it on GitHub
#2904 (comment).


Reply to this email directly or view it on GitHub:
#2904 (comment)

parkr added a commit that referenced this pull request Nov 20, 2014

@parkr parkr merged commit c0951b0 into jekyll:master Nov 20, 2014

1 check passed

continuous-integration/travis-ci The Travis CI build passed
Details

parkr added a commit that referenced this pull request Nov 20, 2014

@XhmikosR XhmikosR deleted the XhmikosR:mixins branch Nov 20, 2014

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

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