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

Transitions and animations #50

Open
3 tasks
ZacSweers opened this issue Sep 3, 2022 · 4 comments
Open
3 tasks

Transitions and animations #50

ZacSweers opened this issue Sep 3, 2022 · 4 comments
Labels
discussion Open discussions! enhancement New feature or request

Comments

@ZacSweers
Copy link
Collaborator

ZacSweers commented Sep 3, 2022

We have a few needs in this space! Currently, we offer some animation APIs via the backstack APIs, but let's try to flush this out into a full API.

  • Inspired by @oldergod's mention in his Broadway talk at Droidcon NYC, let's explore implementing an AnimationFactory API to allow providing animations between two given screens. This would allow us to contribute a transition for these screens as keys.
  • Batteries-included animations. We have one already where it defaults to CrossFade, let's make a few more available like PushLeft, PushRight, etc.
  • Is the current backstack animation API good enough for our needs? Do we want to make other changes to better support an AnimationFactory API? What about looking at what https://github.com/rjrjr/compose-backstack does?

A good example for this would be to make the petlist -> detail transition use the iamge as a shared hero element

@ZacSweers ZacSweers added enhancement New feature or request discussion Open discussions! labels Sep 3, 2022
@ZacSweers ZacSweers added this to the GA milestone Sep 3, 2022
@ZacSweers
Copy link
Collaborator Author

Another option to consider: https://github.com/skydoves/Orbital

@ZacSweers
Copy link
Collaborator Author

Ran into this while using this in CatchUp, something to be mindful of: https://issuetracker.google.com/issues/266401829

ZacSweers added a commit that referenced this issue Feb 2, 2023
This is a breaking API change to `Ui.Content()` to add a `Modifier`
parameter. There are breaking changes, but if using code gen it's fairly
minimal.

```diff
public interface Ui<UiState : CircuitUiState> {
-  @composable public fun Content(state: UiState)
+  @composable public fun Content(state: UiState, modifier: Modifier)
}
```

We've been getting stuck on this for awhile now when trying to bridge to
UI land and finally pulling the trigger on this to ease a bunch of
things.

At a high level, this replaces the toe-hold `ScreenUi` we've been
keeping around as modifiers give us a means of plumbing down extra UI
metadata, so I've removed it from the API.

We can also offer a default onUnavailableContent API now, so I've made
it non-nullable in `CircuitConfig` using that default. If people want
the previous erroring behavior, they can still achieve this with a
custom function of their own.

One place this hits friction is UI libraries that don't use Modifiers
(i.e. Mosaic), but it's easy enough to just ignore that parameter coming
from `ui { state, _ -> ... }`.

Other things
- Realized our code gen artifact was sorta halfway multiplatform and
resulted in confusing KSP configuration errors. Cleaned that up and made
it just a standard JVM artifact.
- Reworked how we generate UI factories a bit in code gen to make it
simpler to maintain. Put a thorough diagram in for good measure.
- Removed redundant wrapper `Box`es that were shims for this
functionality missing.
- It's not possible in abstract composable functions to add a default
value. i.e. can't write `modifier: Modifier = Modifier` in the
`Ui.Content(...)` signature.

This does _add_ the compose UI dependency to circuit core, but after a
lot of thought I think this is ok. Our android artifact already imposed
it for all intents and purposes, this just drops that facade and
embraces the fact that this is a UI architecture and not just a business
logic layer using compose runtime only.

Supersedes #406.
Helps #50 too.
Eases state sharing via modifiers. It would be easy to build modifiers
that build upon each other with this and we no longer have a missing UI
link.
@ZacSweers
Copy link
Collaborator Author

Someone sent me this as a good source of inspiration: https://medium.com/airbnb-engineering/motion-engineering-at-scale-5ffabfc878

ZacSweers added a commit that referenced this issue Jun 24, 2023
This adds a new image viewer in the STAR sample app. This gives us a
good testing ground for shared element transitions (#50) and also cleans
up a bit of the edge to edge behavior in the app.


https://github.com/slackhq/circuit/assets/1361086/5a343f95-395a-4c90-b4ec-de226f124387

Resolves #129
@ZacSweers
Copy link
Collaborator Author

Another thing to keep an eye on: shared element transitions: https://android-review.googlesource.com/c/platform/frameworks/support/+/2499518

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Open discussions! enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants