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

[css-easing-2] linear() custom easing #7484

Merged
merged 25 commits into from Jul 17, 2022
Merged

[css-easing-2] linear() custom easing #7484

merged 25 commits into from Jul 17, 2022

Conversation

jakearchibald
Copy link
Contributor

This PR defines linear(), a way to create custom easings, as discussed in #229 (comment).

In addition, I tweaked the diagrams so they respect dark mode.

Due to various issues, this PR existed in other forms previously:

I think this is ready to land. Once it does, I'll ensure issues are filed with Firefox/Safari/Chrome.

which may be needed for slower animations.
</div>

A <dfn export>linear easing function</dfn>
Copy link
Member

Choose a reason for hiding this comment

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

Would you mind moving this normative text to the top of the section, and having all the examples below?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How much of it do you want above the examples? Everything from here until ### Syntax ###?

css-easing-2/Overview.bs Show resolved Hide resolved
css-easing-2/Overview.bs Outdated Show resolved Hide resolved
css-easing-2/Overview.bs Show resolved Hide resolved

Note: Although this produces a [=linear easing function=],
it serializes as `linear`,
as per the rules in [Serialization](#serialization).
Copy link
Member

Choose a reason for hiding this comment

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

Do we want a literal linear(0, 1) to serialize as linear? I think that would be surprising. I'd prefer to not collapse the keyword and function together; just saying that linear acts the same as linear(0, 1) should be fine imo.

<style>
@media (prefers-color-scheme: dark) {
.easing-graph {
background: none;
Copy link
Member

Choose a reason for hiding this comment

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

While you converted some of the SVGs to have opaque backgrounds, others you've newly added are still transparent, so this is actually making them less readable than the default background: white would do. I think you either want to remove this or make all the SVGs opaque.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My intention was to remove the backgrounds so the graphs could be placed in notes/examples without being in their own visual 'box'.

As in, render like this:

Screen Shot 2022-07-12 at 09 14 06

Rather than this:

Screen Shot 2022-07-12 at 09 14 17

But you would prefer the latter? (my preference isn't strong here so I'm happy to change it)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tabatkins ping. This is the last bit I'm unsure about.

Copy link
Member

Choose a reason for hiding this comment

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

Your screenshot shows them against the lightmode note styling. Have you checked it against the darkmode note styling to confirm that it's actually readable? I suspect it's not. (I'm fine with removing the background in lightmode, since they're authored to be readable against a light background.) I'm mostly just confused why you decided you needed to add an opaque background to some images, but explicitly remove an opaque background from others, when they look fairly similar in color usage to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use dark mode by default so I checked it there too (part of this PR is making those images work well in dark mode). The lack of consistency in the backgrounds is an error on my part, I'll fix them so they're all transparent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here it is against darkmode styling. Seems readable to me.

Screen Shot 2022-07-17 at 11 12 21

Copy link
Contributor

@birtles birtles 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 with Tab's suggestions incorporated.

I agree that we probably want linear(0, 1) to serialize as linear(0, 1).

jakearchibald and others added 2 commits July 12, 2022 09:20
Co-authored-by: Tab Atkins Jr. <jackalmage@gmail.com>
@jakearchibald
Copy link
Contributor Author

jakearchibald commented Jul 12, 2022

@tabatkins @birtles In terms of serialization, I think the normative text is correct.

  • The keyword values ''ease'', ''linear'', ''ease-in'', ''ease-out'', and ''ease-in-out'' are serialized as-is, that is, they are not converted to the equivalent ''cubic-bezier()'' or ''linear()'' function before serializing.

  • Step easing functions, (…clipped for brevity…)

  • A linear easing function created via ''linear()'' is serialized by getting its serialized computed value.

It explicitly says that linear should serialize as-is, and not be converted to the equivalent linear() function. Only functions created via linear() are serialised to the function form.

So:

  • linear serializes as linear
  • linear(0, 1) serializes as linear(0 0%, 1 100%)

I think that's the behaviour we're agreeing on, so I guess the note is misleading?

@birtles
Copy link
Contributor

birtles commented Jul 12, 2022

So:

* `linear` serializes as `linear`

* `linear(0, 1)` serializes as `linear(0 0%, 1 100%)`

I think that's the behaviour we're agreeing on, so I guess the note is misleading?

Yes, looks good to me.

@jakearchibald
Copy link
Contributor Author

I've tried to make the note a bit clearer:

Note: Although this produces a [=linear easing function=], uses of the keyword ''linear'' always serialize as-is, to ''linear''. Whereas the function equivalent ''linear(0, 1)'' will serialize to ''linear(0 0%, 1 100%)''. These rules are in Serialization.

@jakearchibald
Copy link
Contributor Author

Implementation bugs:

@syncbot syncbot restored the css-easing-2-linear branch July 17, 2022 20:48
@plinss plinss deleted the css-easing-2-linear branch July 18, 2022 00:46
@cdoublev
Copy link
Collaborator

cdoublev commented Jul 19, 2022

<easing-function> = linear | linear() | <cubic-bezier-easing-function> | <step-easing-function>

I think linear() should be <linear()> or linear(<linear-stop-list>) otherwise a grammar parser will see it as a function named linear with no arguments.

@jakearchibald
Copy link
Contributor Author

@cdoublev I think this is fixed in b5cf7d9. Let me know if it still isn't right – I don't have a lot of experience with this particular syntax.

@cdoublev
Copy link
Collaborator

I think it is fixed, thanks. I do not really know when a CSS type or an inline value definition should be used, or if it matters at all. <color> is defined with <rgb()> | <rgba()> | <hsl()> | .... The value definition of a CSS type (production) should be unique whereas inline definitions are free to use different function arguments in different places.

A workaround for me could be to assume that a function with no argument is a CSS type reference that is not wrapped in angled brackets, but I would rather not do that, even if there is currently no CSS function that takes no argument (I think).

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

4 participants