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

feat : enable location button in the googleMap #294

Merged
merged 6 commits into from
Dec 19, 2024

Conversation

Ismaillat
Copy link
Contributor

@Ismaillat Ismaillat commented Dec 18, 2024

This Pull Request addresses issue #252.

Description:

This PR introduces critical improvements to the GoogleMap screen, enhancing its usability and functionality:

Enable Location Button:

A new button has been added to the GoogleMap screen, allowing users to enable location services directly from the app.
This provides a seamless way for users to grant location permissions and resolve issues with location services without having to exit or reload the screen.

Location Update Handling:

Resolves the issue where users had to reload the GoogleMap screen to update their location after granting location permissions.
The app now dynamically handles location updates, ensuring the GoogleMap screen reflects the user's current position as soon as location services are enabled.

Restricted Functionality Without Location:

Ensures users cannot take pictures unless their location is enabled and updated.
This prevents location-dependent features, like geotagging, from being used without accurate location data, improving data integrity and user experience.

@Ismaillat Ismaillat linked an issue Dec 18, 2024 that may be closed by this pull request
Copy link
Contributor

@othbcq othbcq left a comment

Choose a reason for hiding this comment

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

This PR effectively resolves the reported issue by ensuring that users cannot attempt to take pictures without granting location permissions. The adoption of modern permission handling techniques and the enhancement of the user interface contribute to a more robust and user-friendly application.

However, incorporating the following improvements will further enhance the overall quality of the codebase.


1. Remove Unused Constants

The LOCATION_PERMISSION_REQUEST_CODE constant is no longer necessary since you've adopted rememberLauncherForActivityResult for handling permissions. Removing unused constants helps keep the codebase clean and maintainable.

// Remove this line
private const val LOCATION_PERMISSION_REQUEST_CODE: Int = 1001

2. Clarify testNoLoca Parameter

The testNoLoca parameter appears to be intended for testing scenarios where location services are disabled. It's important to document its purpose clearly or consider alternative approaches such as dependency injection or mocking for testing purposes.


3. Code Duplication

The FirebaseAuth.getInstance() method is called twice to obtain the auth and user instances. To avoid redundancy and improve efficiency, store the instance once and reuse it.

// Consolidate FirebaseAuth instance retrieval
val auth = remember { FirebaseAuth.getInstance() }
val isLoggedIn = auth.currentUser != null
val user = auth.currentUser // Reuse the same instance
val userEmail = user?.email ?: ""

// Remove the redundant call
// val user = FirebaseAuth.getInstance().currentUser

Great job overall !

@Ismaillat
Copy link
Contributor Author

This PR effectively resolves the reported issue by ensuring that users cannot attempt to take pictures without granting location permissions. The adoption of modern permission handling techniques and the enhancement of the user interface contribute to a more robust and user-friendly application.

However, incorporating the following improvements will further enhance the overall quality of the codebase.

1. Remove Unused Constants

The LOCATION_PERMISSION_REQUEST_CODE constant is no longer necessary since you've adopted rememberLauncherForActivityResult for handling permissions. Removing unused constants helps keep the codebase clean and maintainable.

// Remove this line
private const val LOCATION_PERMISSION_REQUEST_CODE: Int = 1001

2. Clarify testNoLoca Parameter

The testNoLoca parameter appears to be intended for testing scenarios where location services are disabled. It's important to document its purpose clearly or consider alternative approaches such as dependency injection or mocking for testing purposes.

3. Code Duplication

The FirebaseAuth.getInstance() method is called twice to obtain the auth and user instances. To avoid redundancy and improve efficiency, store the instance once and reuse it.

// Consolidate FirebaseAuth instance retrieval
val auth = remember { FirebaseAuth.getInstance() }
val isLoggedIn = auth.currentUser != null
val user = auth.currentUser // Reuse the same instance
val userEmail = user?.email ?: ""

// Remove the redundant call
// val user = FirebaseAuth.getInstance().currentUser

Great job overall !

Thank you for your feedback. I have implemented the requested changes and am now waiting for your feedback.

Copy link
Contributor

@othbcq othbcq left a comment

Choose a reason for hiding this comment

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

Perfect, the changes have been implemented and everything is clean. Approved for merge.

@Ismaillat Ismaillat merged commit 1bdedab into main Dec 19, 2024
3 checks passed
@Ismaillat Ismaillat deleted the fix/google-map-location-request branch December 19, 2024 16:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix: fix google map location request and update
2 participants