Swent-team-6 / icebreakrr

2 stars 1 forks source link

Fix/navigation fix #127

Closed Kapestian closed 1 week ago

Kapestian commented 1 week ago

Navigation fix

what's new

This pull request is a simple fix to the two following bugs:

Implementation

I created the utility function handleSafeBackNavigation in section.shared.NavigationUtils.kt file to generalize the behavior of going back when a confirmation may appear. This function is then used both in ProfileEdit.kt and Filter.kt when clicking on our app's back arrow and in a so called BackHandler that adds the desired behavior to the system "back button"

To go further

It may be a good idea to implement the "press back again to quit the app" Toast to avoid frustration, because right now going back from a top level almost feels like getting catapulted out of the app (it is the desired navigation behavior, but it could be better).

sonarcloud[bot] commented 1 week ago

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
94.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Kapestian commented 1 week ago

Unsatisfying PR when it's too good and I can't make remarks :/

The only thing that comes to my mind is that you could try to make a custom BackHandler component it's always the same format of

BackHandler {
   handleSafeBackNavigation(
       isModified = isModified,
       setShowDialog = { showDialog = it },
       tagsViewModel = tagsViewModel,
       navigationActions = navigationActions)
 }

Thanks for your review. Actually I understand the will to create a special BackHandler but as handleSafeBackNavigation is used for the other back button, it has to exist as a function, moreover creating a " SafeBackHandler " would be overkill as adding a new file just to save two lines in the codebase is not worth it.