echo-swent / echo

4 stars 0 forks source link

integrated camera feature #385

Closed srsingh04 closed 3 months ago

srsingh04 commented 3 months ago

Integrated a camera feature through which now the user can take a take a picture/selfie and upload it as their profile picture. Changing back the ProfileCreationviewModel back to an old version as well to fix a bug in the UI update.

srsingh04 commented 3 months ago

I believe the 'edit' overlay on the profile picture (in the profile screen) shouldn't be there. The user would expect said button to trigger the camera, but there's already a camera button that does that.

This part of the feature has been implemented by @unglazedstamp and I think how it differs from the camera is for being solely there as a functionality to select a picture from the personal gallery of the user. Nonetheless, I personally agree with your idea and think it's actually better to display the profile picture in large and clear and maybe put the edit icon on the circumference of the profile picture circle. This can be an interesting task for a future PR!

alejandrocalles commented 3 months ago

I understand. Is it possible to remove the camera button and connect the camera launcher to the edit button instead? It's just weird to have a non-functional button in production code.

srsingh04 commented 3 months ago

It's just weird to have a non-functional button in production code.

The button is not completely non functional as such...it allows the user to choose amongst multiple images which they have already stored in the past, along with the ones they have clicked using the camera. The feature may come out as a bit confusing to the users in the beginning but unfortunately due to time constraints, I prefer not to touch the code furthermore in the PR. I agree that a dialog box linking it with the camera and a choice of choosing from the gallery would definitely make wayyyy more sense. Thank you for your feedback regardless!

violoncelloCH commented 3 months ago

I squashed all the commits to remove all the "debug" and similar commit messages. Tested and works on my phone, so should be ready to merge.

sonarcloud[bot] commented 3 months ago

Quality Gate Failed Quality Gate failed

Failed conditions
57.5% Line Coverage on New Code (required ≥ 80.0%)

See analysis details on SonarCloud