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

Feature: Attach Photos Captured By Device Camera #138

Merged
merged 5 commits into from
Oct 11, 2023

Conversation

Ismail-AD
Copy link
Contributor

@Ismail-AD Ismail-AD commented Oct 9, 2023

Issue #86
Hello @aritra-tech ,

I hope you're doing well. I wanted to bring to your attention a new pull request I've submitted that introduces a valuable feature to the project.

Pull Request Title: Adding Camera Library for Image Attachment Feature

Description:

In this PR, I have added the CameraX library to project, allowing users to capture and attach images to their notes.

Video
I have attached Video that demonstrate the new feature in action:

imageLibbCheck.mp4

Request for Review:

I request you to review this PR at your earliest convenience. If you have any feedback, suggestions, or questions, please feel free to let me know.

Thank you for your time and consideration. I look forward to your feedback.

@aritra-tech aritra-tech changed the base branch from master to develop October 9, 2023 19:25
@aritra-tech
Copy link
Owner

Hey, @Ismail-AD can you please fix the ktlint check?
Just run ./gradlew ktlintCheck --continue in your terminal and check which lines are showing the error and fix it.

@Ismail-AD
Copy link
Contributor Author

@aritra-tech bro I tried your mentioned command and tried almost every solution but nothing works EVEN WHEN APP IS LAUNCHING and WORKING very FINE.I have attached the ScreenShot can you help me to solve it out :
error

@aritra-tech
Copy link
Owner

Okay let me have a look.

@aritra-tech
Copy link
Owner

I just checked your code and ran it the result I was expecting is not matched. I wanted that there would be a camera option that would enable the user to take images using their camera and when clicked it would be shown in the notes.
But what you have done is you are telling the user that after clicking the image from your camera you have to again go to the Add Image option and add that image which has been taken which is a wrong flow.

I hope you got the point @Ismail-AD what will you do now and what will be the flow?

@Ismail-AD
Copy link
Contributor Author

I got you bro 😀

@Ismail-AD Ismail-AD closed this Oct 11, 2023
@Ismail-AD Ismail-AD reopened this Oct 11, 2023
@aritra-tech
Copy link
Owner

Can you please fix the conflicts @Ismail-AD

@Ismail-AD
Copy link
Contributor Author

okay i will check them

@Ismail-AD
Copy link
Contributor Author

@aritra-tech i will suggest you to do test it and perform next action

@aritra-tech
Copy link
Owner

@aritra-tech i will suggest you to do test it and perform next action

Yeah sure I will test it out and merge it soon if every thing went well.

@Ismail-AD
Copy link
Contributor Author

@aritra-tech check is again pointing to problem which i told you yesteraday

@aritra-tech
Copy link
Owner

Required changes @Ismail-AD

WhatsApp Image 2023-10-11 at 22 05 40_505aa790

  • So in this image, as you can see, there are two notes the second one with title Testing in that note I have added images from my gallery and saved it and in the first note with title Camera I have added images from my camera but when I save those images in the card view images are same the images that I have clicked using my camera is getting replaced witht the images in the galley in the card view but when I click on the first note it shows me the correct images.
    So look into this and do a thorough testing with all the possibilities you will get to know the bugs.
  • When we are taking images from camera it shows us a toast that Photo Attached Successfully, you can do one thing after clicking any image show that toast and navigate back the user to the AddEditScreen.

I hope you got the point, take your take no need to hurry. 😊

@Ismail-AD
Copy link
Contributor Author

Ismail-AD commented Oct 11, 2023

@aritra-tech Bro WHEN i clicked images from camera as per your Image View Code i have to pass URI ok so i neeed to preserve my camera clicked image somewhere and that image is saving to gallery first after that i got the URI for that image as it is saved somewhere (in gallery) and that URI is then used to display image SECONDLY you open gallery to pick image you see that camera clicked image into GALLERY . AM I RIGHT OR WRONG (looking forward towards your clear statement for change If i am wrong and if right this is the way i need to genrate uri i am looking forward towards better suggestions as well)

@aritra-tech
Copy link
Owner

@aritra-tech Bro WHEN i clicked images from camera as per your Image View Code i have to pass URI ok so i neeed to preserve my camera clicked image somewhere and that image is saving to gallery first after that i got the URI for that image as it is saved somewhere (in gallery) and that URI is then used to display image SECONDLY you open gallery to pick image you see that camera clicked image into GALLERY . AM I RIGHT OR WRONG (looking forward towards your clear statement for change If i am wrong and if right this is the way i need to genrate uri i am looking forward towards better suggestions as well)

Yeah got your point I was thinking differently.
But what about the same pictures showing up in the home screen

@Ismail-AD
Copy link
Contributor Author

Ismail-AD commented Oct 11, 2023

You mean user click images from camera(image is preserved in gallery) and save a note and in next note user select image from gallery that is previously preserved image clicked to generate URI (so after saving second note both will have same images there is no exception in my point as per case it is OKAY, as image CLICKED through camera will definitely have to save somewhere) IF i am wrong again i am saying clear the change request statement

@aritra-tech
Copy link
Owner

Actually, I was wrong.
The main problem is saving the images as URI can you do one thing can we change the type of saving image from URI to Bitmap?

@Ismail-AD
Copy link
Contributor Author

Ismail-AD commented Oct 11, 2023

actually give me a second i will perform some conditional checks and then we continue our conversation i have to clear it today so be active

@Ismail-AD
Copy link
Contributor Author

@aritra-tech as far as if we will try to change type which i will not suggest 2 things will happen we have to update Note Model to add list of bitmaps + Storing bitmaps as byte arrays INTO DATABASE can significantly increase the size of your database, leading to performance issues, increased storage requirements and Retrieving image data from a database can be slower AND i will not suggest this rather we should go with previous approach (i am looking forward towards your response ).

@aritra-tech
Copy link
Owner

@aritra-tech as far as if we will try to change type which i will not suggest 2 things will happen we have to update Note Model to add list of bitmaps + Storing bitmaps as byte arrays INTO DATABASE can significantly increase the size of your database, leading to performance issues, increased storage requirements and Retrieving image data from a database can be slower AND i will not suggest this rather we should go with previous approach (i am looking forward towards your response ).

Okay your points are right. Let's then stick to URI only.
I will be merging your PR after some time.
Thanks for contributing @Ismail-AD

Copy link
Owner

@aritra-tech aritra-tech left a comment

Choose a reason for hiding this comment

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

LGTM 🚀
Thanks for contributing @Ismail-AD 🎉

@aritra-tech aritra-tech merged commit 7f5b629 into aritra-tech:develop Oct 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants