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

Updated CSS Values 5 #261

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from
Open

Updated CSS Values 5 #261

wants to merge 3 commits into from

Conversation

SebastianZ
Copy link
Collaborator

I've updated the tests for the CSS Values 5 spec. As there were so many additions and changes, it would be great to get a second pair of eyes on this.

Sebastian

@SebastianZ
Copy link
Collaborator Author

I've updated, extended and fixed the tests of the new functions.

@LeaVerou @Zefling Could you please have a look?

Sebastian

@Zefling
Copy link
Collaborator

Zefling commented Jan 26, 2025

I'm sorry, but I'm really not sure about the validity of the test. The explanation in the spec doesn't seem so easy to understand.

🟥 calc-mix(), cross-fade(), transform-mix() include <progress> = [ <percentage-token> | <number> | <'animation-timeline'> ] && [ by <easing-function> ]? for me is not good, but I not sure.
❓ mix()
🟩 if() only else: case, multiple case are supported
inherit()
🟩 ident(), no test with sibling-index()
ramdom()
toggle() but it would be good to have 2-3 more complex tests

The spec would not have changed on 14 January 2025?

@SebastianZ
Copy link
Collaborator Author

@Zefling Thank you for your response!

I'm sorry, but I'm really not sure about the validity of the test. The explanation in the spec doesn't seem so easy to understand.

That's another reason why I wanted to get a review of my changes.😃 The spec. isn't fully fleshed out yet and is missing examples for most of the new functions.

If I understand the icons you used correctly, then 🟥 means "incorrect",❓ means "unsure", 🟩 means "good but could be improved", and ✅ means "totally fine". Is that correct?

🟥 calc-mix(), cross-fade(), transform-mix() include <progress> = [ <percentage-token> | <number> | <'animation-timeline'> ] && [ by <easing-function> ]? for me is not good, but I not sure.

What do you mean by saying "for me is not good"?

Yes, all those functions include <progress> as their first parameter (plus color-mix() and mix() and potentially also palette-mix(), which is mentioned to be part of those *-mix() functions but currently has a different syntax and is not part of CSS Values 5).
And I added tests for all those functions to cover the different types of values <progress> can take.
Those functions also take a start and and end value as second and third parameter. And the basic idea behind them, as I understand it, is to return an intermediary value between the start and end value, which is calculated using the progress value and an optional easing function.

So, e.g. calc-mix(0.5, 10%, 20%) results in a value of 15%, color-mix(50% in srgb, teal, olive) results in a 50% mixture of those colors, i.e. rgb(64, 128, 64), and transform-mix(--animation-timeline, translateX(10px), translateX(90px)) results in a value equivalent to the progress of the given animation timeline.

Is there something wrong with the tests?

❓ mix()

As I understand it, mix() is basically the generalized version of the other *-mix() functions.

🟩 if() only else: case, multiple case are supported

Right. I'll add tests to cover multiple cases.

🟩 ident(), no test with sibling-index()

I thought I'd skip that because sibling-index() has its own tests, though you're right. It's probably better to include a test for that, as that's also a main use case for that function.

toggle() but it would be good to have 2-3 more complex tests

Ok, I'll add them.

The spec would not have changed on 14 January 2025?

It is unclear to me what you mean by that. That date is when the ident() function was added.

Sebastian

@Zefling
Copy link
Collaborator

Zefling commented Jan 28, 2025

If I understand the icons you used correctly, then 🟥 means "incorrect",❓ means "unsure", 🟩 means "good but could be improved", and ✅ means "totally fine". Is that correct?

Exactly

For example: calc-mix(--scroll-progress-timeline by linear(0, 0.25, 1), 0, 10)
by is usable only with --xx in tests?
All examples are with --scroll-progress-timeline by.
scroll() by <easing-function> case, not possible?

All other fixes ✅ 👍

@SebastianZ
Copy link
Collaborator Author

For example: calc-mix(--scroll-progress-timeline by linear(0, 0.25, 1), 0, 10) by is usable only with --xx in tests? All examples are with --scroll-progress-timeline by. scroll() by <easing-function> case, not possible?

scroll() and view() are also valid. I just thought it is enough to add one <'animation-timeline'> value in combination with and by <easing-function>, as I assume the other ones will be supported as well in implementations.
It would probably be enough to further reduce the tests to a single by <easing-function> test.

What do you think?

Sebastian

@Zefling
Copy link
Collaborator

Zefling commented Feb 6, 2025

Ok, I trust you on this point.

@LeaVerou is it ok for you?

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

Successfully merging this pull request may close these issues.

2 participants