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

WIP: Add functionality to track bitmap information and events #484

Closed
wants to merge 10 commits into from

Conversation

tamimattafi
Copy link

Hello! Thank you for this awesome library!

We are strongly using it for our Story functionality such as on Instagram, and we are willing to help to modify, improve and add more functionality to this library.

Our use-case, that made us create this merge-request, includes the following points:

  • Add the ability to attach buttons/links to a story
  • Add the ability to edit the information of these attachments during creation
  • Add the ability to recreate clickable areas on these attachments and do something if the user interacts

So, changes that we added to the library include these points:

  • Made ViewType a sealed interface instead of enum and used the correct naming conventions for children.
  • Added a new ViewType, called Custom<T>, that takes generic data as a parameter/property, this ViewType can be passed with the bitmap through photoEditor.addImage(bitmap, viewType), if nothing is passed, ViewType.Image will be used by default.
  • Changed OnPhotoEditorListener interface to return the rootView on each event, so we can extract the Rect and other information about the view and its properties
  • OnPhotoEditorListener.onAddViewListener is now called through rootView.post {} to make sure that this event is fired only after the view finishes its creation and setup

For now, this is enough to extract view's data and track it, also to create clickable ghosts during story preview.

I hope this Pull Request will be accepted, it would be awesome to have a review first, so I could adjust any errors since I'm not that well-involved into this library and can miss some moments. Thank you!

CHANGELOG.md Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Owner

@burhanrashid52 burhanrashid52 left a comment

Choose a reason for hiding this comment

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

Thank you for this PR. I think this will open up a lot of customization options for image editing.

However, I am curious to know how clicking implementation will work, since this library saves a static image?

@burhanrashid52
Copy link
Owner

@tamimattafi Also, please make sure all the test passes. There are some failing tests on the current PR.

@burhanrashid52
Copy link
Owner

burhanrashid52 commented Jan 16, 2023

@tamimattafi Are there any updates on this? Because I am planning to release the 3.0.0 version soon. If you can fix this before that then I can merge this in that release.

@github-actions
Copy link

github-actions bot commented Mar 3, 2023

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Mar 3, 2023
@github-actions
Copy link

github-actions bot commented Apr 3, 2023

This PR was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this Apr 3, 2023
@tamimattafi
Copy link
Author

Hello @burhanrashid52,
I apologize for being inactive and not responding to your comments. I was quite busy and didn't really check my notifications.
Thank you for your review, I can fix the issues and add some use-case examples if we can re-open the pull request

@burhanrashid52
Copy link
Owner

Hello @burhanrashid52, I apologize for being inactive and not responding to your comments. I was quite busy and didn't really check my notifications. Thank you for your review, I can fix the issues and add some use-case examples if we can re-open the pull request

Sure. I am reopening it.

@tamimattafi
Copy link
Author

Hello @burhanrashid52, I apologize for being inactive and not responding to your comments. I was quite busy and didn't really check my notifications. Thank you for your review, I can fix the issues and add some use-case examples if we can re-open the pull request

Sure. I am reopening it.

Thank you, I will make sure to do it this weekend.

@tamimattafi
Copy link
Author

Hello @burhanrashid52
I have checked my local code, I've made some changes before your review, fixed some issues and refactored some code. Should I close the Pull Request and open a new one, or I should update the current on with the new changes + add the changes you requested?

@burhanrashid52
Copy link
Owner

Hello @burhanrashid52 I have checked my local code, I've made some changes before your review, fixed some issues and refactored some code. Should I close the Pull Request and open a new one, or I should update the current on with the new changes + add the changes you requested?

If you've changed the majority of the structure then better to open a new PR and if you just address changes regarding my comment then it's better to keep the PR itself to track reviews.

@burhanrashid52
Copy link
Owner

@tamimattafi I am planning to publish a new version 3.0.1. So wanted to check up to see when can we expect the new PR. Or else I publish the new version for now.

@tamimattafi
Copy link
Author

@burhanrashid52 Sorry again for the delay, we have some troubles at work. I think you should publish the new release for now without my changes, I will try to prepare a new PR this weekend (Probably Sunday)

@tamimattafi tamimattafi changed the title Add functionality to track bitmap information and events WIP: Add functionality to track bitmap information and events May 1, 2023
@tamimattafi
Copy link
Author

@burhanrashid52
Hello! I have made some changes to the PR.

New Changes:

  1. Fixed and resolved conversations
  2. Fixed all tests and checked that they are passing (Removed one Unit Test since it's not valid for sealed classes)
  3. Updated the branch with the new changes (61 commits)
  4. Updated AGP to 8.0.0, Gradle to 8.1.1, Kotlin to 1.8.21 and other libraries
  5. Updated java compileOptions
  6. Updated Readme.md and Changelog.md
  7. Update maven publishing scripts, add mavenLocal

Upcoming Changes:

Regarding the new changes I talked about here: Since many commits were made since I opened this PR (61 commits), some changes were cherrypicked manually, the rest of the changes will be moved this week (I will try my best to do that whithin 3 days)

What will be moved/added:

  1. [Breaking] Change OnPhotoEditorListener methods signature, combine the parameters rootView: View?, viewType: ViewType? as ViewReference, this class will be extended for remove and update functionalities (Add tags or unique ids for each added view, for easier manipulation)
  2. [Maybe-Breaking] Use ViewReference instead of raw View inside the PhotoEditor and other components
  3. Add method removeViews to remove specific views with tags or unique ids
  4. Add method updateImage to replace images added by the method addImage
  5. [Breaking] Add styleBuilder or a similar class to the method onEditTextChangeListener for more control, and remove the parameter colorCode
  6. Add possibility to hide borders and remove icons from added views (Helpful to implement swipe down to remove like in Instagram)
  7. Add example of clickable views to demonstrate the usage of the new methods, and to demonstrate ViewType.Custom<T>

@github-actions github-actions bot removed the Stale label May 3, 2023
@github-actions
Copy link

github-actions bot commented Jun 3, 2023

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the Stale label Jun 3, 2023
@github-actions
Copy link

github-actions bot commented Jul 3, 2023

This PR was closed because it has been stalled for 5 days with no activity.

@github-actions github-actions bot closed this Jul 3, 2023
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.

2 participants