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

Keyframe mixin #1640

Closed
ZoeBijl opened this issue Nov 5, 2013 · 20 comments
Closed

Keyframe mixin #1640

ZoeBijl opened this issue Nov 5, 2013 · 20 comments

Comments

@ZoeBijl
Copy link

ZoeBijl commented Nov 5, 2013

I'm trying to create a keyframes mixin for my framework. But it's not working for a number of reasons.

What I'm trying to achieve is write somethig like this:

.keyframes(fill-glass, ~"from { opacity: 1; } to { opacity: 0; }");

And get something like this:

@-webkit-keyframes fill-glass {
    from { opacity: 1; } to { opacity: 0; }
}

@-moz-keyframes fill-glass {
    from { opacity: 1; } to { opacity: 0; }
}

@keyframes fill-glass {
    from { opacity: 1; } to { opacity: 0; }
}

So I wrote this bit of LESS:

.keyframes(@name, @animation) {
    @-webkit-keyframes @name {
        @animation
    }
    @keyframes @name {
        @animation
    }
}

And those who know the inner workings of LESS probably see a lot wrong here, first of all @-webkit-keyframes and @Keyframes are never defined.

@name is, but is not used for some reason, probably because it crashes on @Keyframes. A stand alone variable like @animation doesn't work either.

Is this at all possible?

@10xLaCroixDrinker
Copy link

This is the best I could get last time I tried something like this: https://github.com/jking90/zen-garden/blob/master/styles/common/animation.less

@ZoeBijl
Copy link
Author

ZoeBijl commented Nov 5, 2013

That is what I have now :) But using that you are still repeating yourself.

@seven-phases-max
Copy link
Member

Since it's not really an issue, stackoverflow.com would be a better place to ask a question like this (I.e. "How-To"). Though honestly speaking I suspect further disscusion will reveal certain @keyframes related issues and/or a feature request.

@ZoeBijl
Copy link
Author

ZoeBijl commented Nov 5, 2013

Well, Stack Overflow would be fine, but I think this has something to do with how LESS parses variables.

@seven-phases-max
Copy link
Member

OK, so here's simple answer: neither @keyframes @name nor { @animation } are valid LESS statements. So error is expected (Hence why I suggested stackoverflow as a faster way to find possible solution for this task).

@ZoeBijl
Copy link
Author

ZoeBijl commented Nov 6, 2013

Okay, here is the follow-up question: why not? Is there a technical reason?

@seven-phases-max
Copy link
Member

There're two distinct problems:

  1. @keyframes @name: LESS does not support using a variable as a @keyframe name. In fact, the @keyframes support in LESS is as simple as just plain CSS @keyframes (no advanced features like selector interpolation or nesting work with @keyframes). This feature is probably a good candidate for a feature request.
  2. { @animation }: Variables in LESS just do not work this way. You cannot put a variable anywhere and expect it to be replaced by its contents. In other words, LESS variables are not macros. The closest LESS concept to what you're trying to achieve with { @animation } is a "Mixin".
    (Although it is possible to force LESS to expand a variable like an arbitrary text macro inside a ruleset block via certain escape hacks, this obviously is not a technics to ever be recommended unless vitally necessary.)

See also #1264, #1506, #965.

@kuus
Copy link

kuus commented Nov 12, 2013

Hello, I wrote this keyframes/animation less mixin lately, if you're intrested it's here https://github.com/kuus/animate-me.less

@lukeapage
Copy link
Member

There are issues for allowing mixins to accept blocks, which is for this
purpose.. combined with allowing variables for the name, this would allow
what you are asking.

@thybzi
Copy link

thybzi commented Nov 29, 2013

You can also use my solution to generate CSS keyframes with LESS: https://github.com/thybzi/keyframes

@seven-phases-max
Copy link
Member

So, speaking of keyframes... What name interpolation syntax we would choose?
Could be:

  • @keyframes @var {...} - this syntax used for interpolation in @media queries (it should not cause any conflicts with CSS unless they invent something new like @keyframes @banana {...})
  • @keyframes @{var} {...} - as used for other variable interpolations.

@matthew-dean
Copy link
Member

LESS does not support using a variable as a @Keyframe name.

It should.

{ @animation }: Variables in LESS just do not work this way.

They should. It was once arbitrary that variables only correctly evaluated when used for property values. Then they were expanded to selector names, properties, and mixins. There's no reason why variable replacement could / should not work anywhere in your statement, and the fact that it doesn't is IMO what's causing this pile of block inclusion workarounds and ongoing submitted issues. See #1648 (plus the ones you listed).

@seven-phases-max
Copy link
Member

They should.

But not nesessary that way.
.keyframes(fill-glass, ~"from { opacity: 1; } to { opacity: 0; }"); there the @animation parameter becomes just a macro with no syntax checking.
Also simple @{animation}; or even @animation; looks more like a typo rather then a block (aka "mixin in a variable") expansion. Well, #965 is looong, there a lot of examples, syntax suggestions and their problems :)

@matthew-dean
Copy link
Member

I'm just pointing out that we're interpreting variables in literally every other place within a declaration except for this one case, so it's now a case of an arbitrary omission.

I agree that this is not desirable: .keyframes(fill-glass, ~"from { opacity: 1; } to { opacity: 0; }");

@lukeapage
Copy link
Member

This is the syntax I've been thinking about a while to solve this and other issues.

.keyframes(@name) {
    @-webkit-keyframes @name {
        @content;
    }
    @keyframes @name {
        @content;
    }
}

:wrap-with(.keyframes("opac-change")) {
  from { opacity: 1; }
  to { opacity: 0; }
}

@lukeapage
Copy link
Member

one upside is that from a library perspective it is backwards compatible. only when a wrapped mixin is called would you need to know it was special.

@lukeapage
Copy link
Member

see #1859

sparanoid added a commit to sparanoid/path-menu that referenced this issue Mar 10, 2014
See less/less.js#1640 for more info about LESS loop support for @kayframes
@wallaroo
Copy link

hi!
i have this mixin definition in a library file 'animation.less'

.keyframe(@name, @roules) {
  @-webkit-keyframes @name {
    @roules();
  }
  @-moz-keyframes @name {
    @roules();
  }
  @-ms-keyframes @name {
    @roules();
  }
  @-o-keyframes @name {
    @roules();
  }
  @keyframes @name {
    @roules();
  }
}

if i import this file normally (@import 'animaton.less') all works well, but if i set the import as reference (@import (reference) 'animaton.less') my uses of the mixin doesn't work!
the compilation runs without errors but the result is empty.
it's correct or is a bug?

@seven-phases-max
Copy link
Member

it's correct or is a bug?

It's more like "something that was not considered when reference is implemented".

@jonschlinkert
Copy link
Contributor

yeah, it's a feature request

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

No branches or pull requests

9 participants