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

[FR]: NiaApp background composable could be improved #349

Open
2 tasks done
dturner opened this issue Oct 21, 2022 · 2 comments
Open
2 tasks done

[FR]: NiaApp background composable could be improved #349

dturner opened this issue Oct 21, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@dturner
Copy link
Collaborator

dturner commented Oct 21, 2022

Is there an existing issue for this?

  • 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

  • I agree to follow this project's Code of Conduct
@emitchel
Copy link

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.

@alexvanyo
Copy link
Contributor

alexvanyo commented Dec 1, 2022

The original comment was for the non-standard setup for the background that looks like val background: @Composable (@Composable () -> Unit) -> Unit = // ...: #330 (comment)

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.

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

No branches or pull requests

3 participants