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

Fix the QE recomposition when selecting an avatar #534

Merged
merged 1 commit into from
Jan 15, 2025

Conversation

AdamGrzybkowski
Copy link
Contributor

@AdamGrzybkowski AdamGrzybkowski commented Jan 13, 2025

Description

There's a bug that's causing the QE to reload on Avatar change. The problem is this line. When an avatar is selected the onAvatarSelected lambda is invoked and this triggered the recomposition caused by the changed startDestination.

Changing the start destination was convenient but not necessary, so I reworked the code to not require it.

Testing Steps

  1. Launch the QE
  2. Select an avatar
  3. Confirm the QE did not recompose
  4. Test for regression in the back button navigation - Go through the OAuth flow, Alt text page, etc.

@AdamGrzybkowski AdamGrzybkowski added Bug Something isn't working [Feature] Gravatar-Quickeditor Gravatar Quick Editor module labels Jan 13, 2025
@AdamGrzybkowski AdamGrzybkowski added this to the 2.3.0 milestone Jan 13, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 13, 2025

📲 You can test the changes from this Pull Request in Gravatar Demo by scanning the QR code below to install the corresponding build.
App Name Gravatar Demo
Commit4cfb02e
Direct Downloadgravatar-demo-prototype-build-pr534-4cfb02e.apk

Copy link
Contributor

@hamorillo hamorillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested the PR, it works as expected.

When an avatar is selected the onAvatarSelected lambda is invoked and this triggered the recomposition caused by the changed startDestination.

Can you explain this a little bit more, please. I don't fully understand why that was happening.

@AdamGrzybkowski
Copy link
Contributor Author

AdamGrzybkowski commented Jan 14, 2025

Can you explain this a little bit more, please. I don't fully understand why that was happening.

That's a good question. I only have a guess but no concrete answer. The startDestination is part of the Graph that is a param in a NavHost Composable. The onAvatarSelected is a lambda passed from the very top that goes through all the composables and must be invoking the recomposition due to a modified Graph state on its way up.

Copy link
Contributor

@hamorillo hamorillo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀

@AdamGrzybkowski AdamGrzybkowski force-pushed the adam/fix_recomposition_on_avatar_selection branch from a717cb3 to 4cfb02e Compare January 15, 2025 09:01
@AdamGrzybkowski AdamGrzybkowski merged commit 886b673 into trunk Jan 15, 2025
17 checks passed
@AdamGrzybkowski AdamGrzybkowski deleted the adam/fix_recomposition_on_avatar_selection branch January 15, 2025 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working [Feature] Gravatar-Quickeditor Gravatar Quick Editor module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants