oppia / oppia-android

A free, online & offline learning platform to make quality education accessible for all.
https://www.oppia.org
Apache License 2.0
300 stars 502 forks source link

Fix #3258 - Handling Removal of the CircularImageView dependency #5350

Closed Rd4dev closed 3 months ago

Rd4dev commented 4 months ago

Explanation

Fixes #3258

The Pull Request #4155 substituted CircularImageView with ShapeableImageView, yet the dependency persists in the project. This PR addresses the final steps of removing the remaining dependency.

add_shadow and shadow_radius were exclusive to CircularImageView. Upon removing the dependencies, these attributes became irrelevant, so they were omitted from the layouts. Their removal had no impact on the functioning views as they were unrelated to the actual attributes of the working views. [Attached reference images below]

Essential Checklist

For UI-specific PRs only

Having add_shadow

Todo

Remove Dependency from Bazel and External libraries.!

Rd4dev commented 4 months ago

@adhiamboperes PTAL

Rd4dev commented 3 months ago

Hi @adhiamboperes,

The third_party/versions.bzl file appears to be missing the CircularImageView dependency. It seems that PR #2659 might have overlooked the addition of com.jackandphantom.android:circularimageview.

Two dependencies were initially employed for circular images:

Both dependencies were eventually mixed in layouts, with de.hdodenhof.circleimageview.CircleImageView predominantly [ref. PR #4155].

Issue #3258 mentions replacing com.jackandphantom.android:circularimageview, but the comment suggests replacing de.hdodenhof.circleimageview.CircleImageView as well which was implemented in PR #4155.

Seeking clarification on whether it is appropriate to proceed with the removal of de.hdodenhof.circleimageview.CircleImageView, given that none of the layouts appear to utilize this dependency.

adhiamboperes commented 3 months ago

@Rd4dev , please remove the dependency as it is unused.

Rd4dev commented 3 months ago

@adhiamboperes PTAL

oppiabot[bot] commented 3 months ago

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

Rd4dev commented 3 months ago

@adhiamboperes PTAL

oppiabot[bot] commented 3 months ago

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

oppiabot[bot] commented 3 months ago

Unassigning @adhiamboperes since they have already approved the PR.

oppiabot[bot] commented 3 months ago

Assigning @BenHenning for code owner reviews. Thanks!

BenHenning commented 3 months ago

Updating with latest & enabling auto-merge since everything seems to look good.

oppiabot[bot] commented 3 months ago

Unassigning @BenHenning since they have already approved the PR.

oppiabot[bot] commented 3 months ago

Hi @Rd4dev, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to ask someone to merge your PR once the CI checks pass and you're happy with it. Thanks!