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

Fix property-no-unknown false positives for scroll-driven animations #7088

Closed
renato-bohler opened this issue Jul 19, 2023 · 6 comments · Fixed by #7090
Closed

Fix property-no-unknown false positives for scroll-driven animations #7088

renato-bohler opened this issue Jul 19, 2023 · 6 comments · Fixed by #7090
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule upstream relates to an upstream package

Comments

@renato-bohler
Copy link
Contributor

renato-bohler commented Jul 19, 2023

What minimal example or steps are needed to reproduce the bug?

.view-timeline-works {
	view-timeline: --my-name block;
	view-timeline-axis: block;
	view-timeline-inset: 200px;
	view-timeline-name: --my-name;
}

.animation-range-works {
	animation-range: contain 100px;
	animation-range-start: contain;
	animation-range-end: 100px;
}

.animation-timeline-does-not-work {
	animation-timeline: view();
}

What minimal configuration is needed to reproduce the bug?

{
  "extends": [
    "stylelint-config-standard"
  ]
}

How did you run Stylelint?

Demo

Which Stylelint-related dependencies are you using?

{
  "stylelint": "15.10.2",
  "stylelint-config-standard": "34.0.0"
}

What did you expect to happen?

The animation-timeline property to not be unknown.

What actually happened?

The following error gets thrown:

Unexpected unknown property "animation-timeline"  property-no-unknown

Do you have a proposal to fix the bug?

Add animation-timeline to lib/reference/properties.js? Though I'm not familiar with the source for this library, so I don't understand why view-timeline and animation-range properties are not unknown if they don't seem to be defined there.

I also don't comprehend why the view function is not being flagged as unknown as I don't see it being declared in lib/reference/functions.js.

So maybe we should add those as well? If that's the case, I'm happy to open a PR.

@renato-bohler
Copy link
Contributor Author

For more context, in case anyone is curious, I'm using this on my portfolio: renato-bohler/renato-bohler.github.io#92 and this is the GH Actions run that's failing.

@ybiquitous
Copy link
Member

@renato-bohler Thanks for opening this issue with a reproducible demo!

The property-no-unknown rule depends on the known-css-properties package, so changes to upstream would be best.

const properties = require('known-css-properties').all;

Could you please consider feedback to the known-css-properties repository if you're interested?

@ybiquitous ybiquitous added upstream relates to an upstream package status: ready to implement is ready to be worked on by someone type: bug a problem with a feature or rule labels Jul 19, 2023
@ybiquitous ybiquitous changed the title Scroll & View timeline support Fix property-no-unknown false positives for scroll-driven animations Jul 19, 2023
@ybiquitous
Copy link
Member

FYI. The spec is here: https://drafts.csswg.org/scroll-animations/

@renato-bohler
Copy link
Contributor Author

Hmm interesting, I'll definitely forward this to known-css-properties. Should we keep this issue open in the meantime? I can open a PR updating the version of this dependency whenever that gets done.

@ybiquitous
Copy link
Member

Should we keep this issue open in the meantime?

Yes! Let's keep it open so other people can find the same problem.

@jeddy3
Copy link
Member

jeddy3 commented Jul 20, 2023

The spec is here: https://drafts.csswg.org/scroll-animations/

That's a fascinating CSS feature. The animation-timeline property is defined in this spec too. There's also a decent write-up on MDN. It's the first instance I've seen of a <dashed-ident> for a named <custom-ident>.

I also don't comprehend why the view function is not being flagged as unknown as I don't see it being declared in lib/reference/functions.js.

As with the known-css-properties package, we use an upstream package for functions (and it includes view).

The rule depends on the known-css-properties package, so changes to upstream would be best

Agreed. @renato-bohler In the meantime, you can workaround the issue using the ignoreProperties secondary option:

{
  "extends": [
    "stylelint-config-standard"
  ],
  "rules": {
    "property-no-unknown": [true, { "ignoreProperties": ["animation-timeline"] }]
  }
}

Demo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wip is being worked on by someone type: bug a problem with a feature or rule upstream relates to an upstream package
Development

Successfully merging a pull request may close this issue.

3 participants