nowinandroid icon indicating copy to clipboard operation
nowinandroid copied to clipboard

[FR]: NiaApp background composable could be improved

Open dturner opened this issue 1 year ago • 2 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the problem

Context: https://github.com/android/nowinandroid/pull/330/files/ee4eb25ba07e7a017c52f54fa49547a0e733ff8b#r1001237941

From @manuelvicnt: What's this complexity? Could this be a slot API instead? And the content inside it could be using moveableContent so that state isn't lost

Describe the solution

Let's discuss this in our weekly.

Additional context

No response

Code of Conduct

  • [X] I agree to follow this project's Code of Conduct

dturner avatar Oct 21 '22 14:10 dturner

Was this referencing how changing the background gradient by screen makes the navigation more janky? Example, going from ForYou <-> Saved, it's a bit jarring, no soft transition because ForYou has a custom background. Saved <->Interests has a pleasant transition because they have the same background, thus no scaffolding recomposition.

Wonder if there's a better way to go about this, otherwise, we should just keep the background local to the screen.

emitchel avatar Nov 30 '22 06:11 emitchel

The original comment was for the non-standard setup for the background that looks like val background: @Composable (@Composable () -> Unit) -> Unit = // ...: https://github.com/android/nowinandroid/pull/330#discussion_r1001237941

The setup here is a bit tricky, since we want the background to be displayed behind some shared elements (like the nav bar), but it is different depending on the destination like you mentioned.

We should be able to replace the current setup with one that uses movableContentOf, to ensure that the state of the application isn't lost when the background in use changes.

alexvanyo avatar Dec 01 '22 23:12 alexvanyo