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

Add support for transition in CSS api. #2228

Merged
merged 8 commits into from
Sep 12, 2022
Merged

Add support for transition in CSS api. #2228

merged 8 commits into from
Sep 12, 2022

Conversation

Ayfri
Copy link
Contributor

@Ayfri Ayfri commented Aug 4, 2022

This PR adds a builder to create transitions with the CSS API.

@eymar eymar added the web label Sep 7, 2022
@eymar
Copy link
Collaborator

eymar commented Sep 9, 2022

Hi @Ayfri, Thank you for your contribution!

Could you please add unit tests for all new APIs in .../web/core/src/jsTest? Look at TransformTests as an example. We simply verify that the built css string is correct.

Also, since this a new API, we usually mark it with @ExperimentalComposeWebStyleApi annotation. It needs to be added to all new classes and extension functions.

P.S. I apologise for a long time without any answer

@eymar eymar self-requested a review September 9, 2022 08:24
@Ayfri
Copy link
Contributor Author

Ayfri commented Sep 9, 2022

I'm creating the unit tests and facing a problem where we have to decide how the API behave :

transitions {
	duration(1.s)
	delay(.5.s)
	properties("height", "width")
	
	"width" {
		duration(2.s)
	}
}

Should it returns

  1. height 1s 0.5s, width 2s by replacing the width property.
  2. height 1s 0.5s, width 2s 0.5s by modifying the width property.
  3. height 1s 0.5s, width 1s 0.5s, width 2s by just adding the property, behaves same as 1 but has a repetition.

Currently, it has the third behavior.

@eymar
Copy link
Collaborator

eymar commented Sep 9, 2022

@Ayfri

3. height 1s 0.5s, width 1s 0.5s, width 2s by just adding the property, behaves same as 1 but has a repetition.

I think 3rd variant is okay, since it's possible to write such a css manually as well.

@eymar eymar merged commit d85bd8c into JetBrains:master Sep 12, 2022
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.

None yet

2 participants