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

QuickEditor: Load Avatar object from AvatarsRepository in AltTextViewModel #531

Merged
merged 3 commits into from
Jan 13, 2025

Conversation

hamorillo
Copy link
Contributor

@hamorillo hamorillo commented Jan 10, 2025

Closes #523

Description

With the recent implementation of the AvatarStorage we are ready to remove the extra params we needed to open the AltTextPage (url and alt_text).

We can load the avatar data from the repository just by using the avatarId so we can simplify the navigation as we only need the email and the avatarId to open the AltTextPage.

Testing Steps

  • Smoke test the AltText feature (everything should work as before)

@hamorillo hamorillo added [Feature] Gravatar-Quickeditor Gravatar Quick Editor module tech debt labels Jan 10, 2025
@hamorillo hamorillo added this to the 2.3.0 milestone Jan 10, 2025
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 10, 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
Commitef9fc5d
Direct Downloadgravatar-demo-prototype-build-pr531-ef9fc5d.apk

@@ -4,4 +4,6 @@ internal sealed class AltTextAction {
data object AltTextUpdated : AltTextAction()

data object AltTextUpdateFailed : AltTextAction()

data object AvatarCantBeLoaded : AltTextAction()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This case shouldn't occur in the QE, but just in case and as a way to handle it.

@hamorillo hamorillo marked this pull request as ready for review January 10, 2025 12:57
@hamorillo hamorillo force-pushed the hamorillo/523-alttextpage-required-params branch from 59d0f80 to 78dfd27 Compare January 13, 2025 13:13
Copy link
Contributor

@AdamGrzybkowski AdamGrzybkowski left a comment

Choose a reason for hiding this comment

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

:shipit:

We can load the avatar data from the repository just by using the avatarId to remove the extra parameters we used to navigate to the AltTextPage.
@hamorillo hamorillo force-pushed the hamorillo/523-alttextpage-required-params branch from 78dfd27 to ef9fc5d Compare January 13, 2025 15:06
@hamorillo hamorillo merged commit 73f6630 into trunk Jan 13, 2025
17 checks passed
@hamorillo hamorillo deleted the hamorillo/523-alttextpage-required-params branch January 13, 2025 15:20
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.

Quick Editor: Alt Text - Refactor AltText page to reduce the number of requiered params
3 participants