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: Use uCrop fork #348

Merged
merged 3 commits into from
Oct 2, 2024
Merged

QuickEditor: Use uCrop fork #348

merged 3 commits into from
Oct 2, 2024

Conversation

AdamGrzybkowski
Copy link
Contributor

Closes #305

Description

This PR replaces the og uCrop with Autmattic fork.

cc @ParaskP7

Testing Steps

1.Run the Demo App
2. Upload an image from the camera using a real device
3. Verify that the image size is smaller than the original (either stop the app before deleting an image and check in Device Explorer or monitor the API request)

@AdamGrzybkowski AdamGrzybkowski added Bug Something isn't working Feature: gravatar-quickeditor Gravatar Quick Editor module labels Oct 2, 2024
@AdamGrzybkowski AdamGrzybkowski added this to the 2.0.0 milestone Oct 2, 2024
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 2, 2024

📲 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
Commit6e9edb6
Direct Downloadgravatar-demo-prototype-build-pr348-6e9edb6.apk

@@ -25,7 +25,7 @@ dependencyResolutionManagement {
repositories {
google()
mavenCentral()
maven { url = uri("https://jitpack.io") }
maven { url = uri("https://a8c-libs.s3.amazonaws.com/android") }
Copy link

Choose a reason for hiding this comment

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

FYI: This configuration below is what you actually need:

        maven {
            url = uri("https://a8c-libs.s3.amazonaws.com/android")
            content {
                includeGroup("com.automattic")
                includeGroup("com.automattic.ucrop")
            }
        }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm what does this add? It works fine without it 🤔

Copy link

Choose a reason for hiding this comment

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

Good question @AdamGrzybkowski ! 💯

So, the purpose of content/includeGroup is:

  1. Artifact Filtering: The content block defines which artifacts are allowed to be downloaded from this specific repository.
  2. Group Restrictions: By using includeGroup, the configuration limits the repository to only provide artifacts from the specified group IDs.

So, in our case, the repository configuration I shared above will do the following:

  1. Include com.automattic: Allow artifacts with the group ID "com.automattic" to be downloaded from this repository.
  2. Include com.automattic.ucrop: Also permit artifacts with the group ID "com.automattic.ucrop" to be retrieved from this repository.

This setup is in general what we have been applying to all our repos, just to optimize dependency resolution by only checking for the specified groups, prevent unintended artifacts from being downloaded and explicitly define which artifact groups are expected from this particular repository.

As such, this configuration ensures that only artifacts from the com.automattic and com.automattic.ucrop groups will be considered when resolving dependencies from the https://a8c-libs.s3.amazonaws.com/android repository. Any other group IDs will be ignored for this specific repository, even if they exist there.

Does that all make sense to you? 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the explanation, makes total sense!

Copy link

Choose a reason for hiding this comment

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

Awesome, thank YOU for the inquiry! 🥇

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.

:shipit:

@AdamGrzybkowski AdamGrzybkowski merged commit 179c053 into trunk Oct 2, 2024
16 checks passed
@AdamGrzybkowski AdamGrzybkowski deleted the adam/305_ucrop_fork branch October 2, 2024 19:32
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.

QuickEditor: Downsize uploaded image
4 participants