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.

Sign up for GitHub

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

Jump to bottom

Find a maintained replacement for CircularImageView #3258

Closed
BenHenning opened this issue May 27, 2021 · 14 comments · Fixed by #5350
Closed

Find a maintained replacement for CircularImageView #3258

BenHenning opened this issue May 27, 2021 · 14 comments · Fixed by #5350
Assignees
@Rd4dev
Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

The codebase currently depends on 2 different CircularImageView dependencies. One of them, https://github.com/sparrow007/CircularImageview, is no longer maintained and should be replaced with a dependency that's kept up-to-date. This will allow us to remove our custom fork of the library (which was needed per #2923 to remove the codebase's dependency on jcenter).
@BenHenning BenHenning mentioned this issue May 27, 2021
Closed
@anandwana001
Copy link
Contributor

Rajat and I discussed about using ShapeableImageView from material design library.
https://developer.android.com/reference/com/google/android/material/imageview/ShapeableImageView
Need to test if it goes similar with our mocks.

@BenHenning
Copy link
Member Author

SGTM if that works.

@AkinchanKushwaha
Copy link

Hi, Please check out https://github.com/hdodenhof/CircleImageView. It has 13.9k stars on GitHub.

@anandwana001
Copy link
Contributor

@AkinchanKushwaha Currently, we are using this library only, but we are looking forward to move to ShapeableImageView .

@aadityaguptaa
Copy link
Contributor

Can I work on this issue?

@anandwana001
Copy link
Contributor

Hi @aadityaguptaa I had assigned you this issue.
In order to complete this issue please check the above comments and we can go for the best suitable solution.

@aadityaguptaa
Copy link
Contributor

@anandwana001 thank you, I am working on this, will give you updates in a couple of days

@aadityaguptaa aadityaguptaa mentioned this issue Oct 20, 2021
Closed
6 tasks
@rt4914
Copy link
Contributor

rt4914 commented Nov 18, 2021

@aadityaguptaa Unassigning you from this issue because of inactivity, please re-assign yourself if you are currently working on this.

@bkaur-bkj bkaur-bkj assigned bkaur-bkj and unassigned bkaur-bkj Jan 10, 2022
@bhaktideshmukh
Copy link
Contributor

Hey @rt4914 I would like to work upon this issue.Can you please assign it to me?

@bkaur-bkj
Copy link
Contributor

@bhaktideshmukh done

bhaktideshmukh added a commit to bhaktideshmukh/oppia-android that referenced this issue Jan 28, 2022
@bhaktideshmukh
Fix oppia#3258 : Replacing CircularImageView with ShapeableImageView
3e86883
@bhaktideshmukh bhaktideshmukh mentioned this issue Jan 28, 2022
Closed
6 tasks
bhaktideshmukh added a commit to bhaktideshmukh/oppia-android that referenced this issue Feb 2, 2022
@bhaktideshmukh
Fix part of oppia#3258 : Replacing CircularImageView with ShapeableIm…
549b067 
…ageView
@bhaktideshmukh bhaktideshmukh mentioned this issue Feb 4, 2022
Merged
6 tasks
BenHenning pushed a commit that referenced this issue Apr 20, 2022
@bhaktideshmukh
Fix part of #3258 : Replacing CircularImageView with ShapeableImageVi…
5221bfc 
…ew (#4155)

* Fix part of #3258 : Replacing CircularImageView with ShapeableImageView

* Updated bazel version

* Updated maven_install.json

* Added missing appearance overlay property in some files

* Added missing appearance overlay property in some files

* Added comment to style.xml and removed blank line

* Syncing with latest develop branch

* added elevation

* decreased elevation

* decreased elevation and replaced CircularImageView with
ShapeableImageView in nav_header_navigation_drawer.xml

* added comment for style

* empty_commit

* empty_commit

* maven_install

* Update maven_install.json to latest version

* added corner_family

* added comment for shapeableimageview

* changed style name
@Broppia Broppia added the Impact: Low Low perceived user impact (e.g. edge cases). label Jul 29, 2022
@BenHenning BenHenning added Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer and removed good second issue labels Sep 14, 2022
@rt4914 rt4914 moved this from Needs Triage to In Progress: Being implemented in [Team] Core Learner and Mastery flows & UI Frontend - Android Sep 20, 2022
@seanlip seanlip added bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure labels Mar 28, 2023
@adhiamboperes adhiamboperes added Work: High It's not clear what the solution is. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. and removed Work: High It's not clear what the solution is. labels Jun 20, 2023
@adhiamboperes adhiamboperes added the good first issue This item is good for new contributors to make their pull request. label Oct 28, 2023
@dmdbilal
Copy link

dmdbilal commented Jan 6, 2024

Can I work on this issue ?

@Rd4dev
Copy link
Collaborator

Rd4dev commented Feb 24, 2024

Hi @BenHenning @adhiamboperes, I was wondering if there are any remaining tasks related to this issue (as it is still open) that I could assist with. The PR #4155 replaced CircularImageView with ShapeableImageView, and the review suggests renaming the title to tackle a segment of #3258, considering potential future tasks associated with ProfileProgress, ProfileList, and ProfileEdit, but it seems like potential fixes for those are already being included.

I am reaching out to seek clarification on whether there are additional tasks or if removing the CircularImageView dependency is the only remaining aspect to address for this issue.

Thanks

@adhiamboperes
Copy link
Collaborator

@Rd4dev, could you please search the codebase for usages of CircularImageview, and make note of those? If none is found, please test removing the dependency by removing from gradle and building the app(although complete removal means removing from bazel as well).

Rd4dev added a commit to Rd4dev/oppia-android that referenced this issue Mar 1, 2024
@Rd4dev
Fix part of oppia#3258 - Handling Removal of the dependency | Gradle
31322e1
@Rd4dev Rd4dev mentioned this issue Mar 1, 2024
Merged
6 tasks
@Rd4dev
Copy link
Collaborator

Rd4dev commented Mar 1, 2024 •
edited
Loading

@adhiamboperes , Sure! While a majority of occurrences were updated, a few lingering attributes tied to CircularImageView were identified and addressed in the PR #5350. I'll handle the dependency removal from Bazel in the subsequent commits.

Rd4dev added a commit to Rd4dev/oppia-android that referenced this issue Mar 1, 2024
@Rd4dev
Fix part of oppia#3258 - Handling Removal of the dependency | Bazel
01ba99a
Rd4dev added a commit to Rd4dev/oppia-android that referenced this issue Mar 14, 2024
@Rd4dev
Fix part of oppia#3258 - Handling Removal of the dependency | Circle …
0cfccc2 
…Image View
BenHenning added a commit that referenced this issue Mar 20, 2024
@Rd4dev @BenHenning
Fix #3258 - Handling Removal of the CircularImageView dependency (#5350)
d0c8b81 
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->
Fixes #3258 

The Pull Request
[#4155](#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
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
![Having
add_shadow](https://github.com/oppia/oppia-android/assets/122200035/6b5be06f-3ee2-4beb-abc7-e170e8794f67)


### Todo
Remove Dependency from Bazel and External libraries.!

---------

Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Rd4dev Rd4dev

Labels
bug End user-perceivable behaviors which are not desirable. good first issue This item is good for new contributors to make their pull request. Impact: Low Low perceived user impact (e.g. edge cases). Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Projects
[Team] Core Learner and Mastery flows & UI Frontend - Android
Archived in project
Milestone
No milestone