Skip to content

feat: Add a rationale to explain why the user should grant notifications permission#622

Merged
NicolasBourdin88 merged 9 commits intomainfrom
feat/notification-permission-modal
Apr 28, 2026
Merged

feat: Add a rationale to explain why the user should grant notifications permission#622
NicolasBourdin88 merged 9 commits intomainfrom
feat/notification-permission-modal

Conversation

@NicolasBourdin88
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 15, 2026

PR Reviewer Guide 🔍

(Review updated until commit f046119)

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Compose Hook Violation

rememberPermissionManagerState is called inside conditional else blocks in three methods (TopBarDownloadButton, BottomBarDownloadButton, CardDownloadButton). Compose hooks must be called unconditionally at the top level of Composable functions to maintain proper state across recompositions. If the condition changes between recompositions, this will cause crashes or undefined behavior. Hoist the rememberPermissionManagerState call to the top level of each Composable method, or use if (condition) { return } pattern to ensure hooks are always executed in the same order.

        val permissionManagerState = rememberPermissionManagerState(PermissionType.WriteExternalStorage)

        TopAppBarButtons.Download(
            enabled = downloadRequest.isAwaitingCall,
            onClick = permissionManagerState.waitUntilGranted { downloadRequest() },
        )
    }
}

@Composable
fun BottomBarItem(modifier: Modifier = Modifier) {
    if (removalRequest.isAwaitingCall) {
        DownloadStatus { btnData, action, progressIndicator ->
            BottomBarButton(
                icon = btnData.icon,
                labelResId = btnData.labelResId,
                onClick = action,
                enabled = action.isAwaitingCall,
                modifier = modifier,
                extraBackgroundContent = progressIndicator,
            )
        }
    } else {
        val permissionManagerState = rememberPermissionManagerState(PermissionType.WriteExternalStorage)

        BottomBarButton(
            icon = ButtonData.download.icon,
            labelResId = ButtonData.download.labelResId,
            enabled = downloadRequest.isAwaitingCall,
            onClick = permissionManagerState.waitUntilGranted { downloadRequest() },
            modifier = modifier,
        )
    }
}

@Composable
fun CardCornerButton(modifier: Modifier = Modifier) {
    if (removalRequest.isAwaitingCall) {
        DownloadStatus { btnData, action, _ ->
            CardCornerButton(
                icon = btnData.icon,
                labelResId = btnData.labelResId,
                onClick = action,
                enabled = action.isAwaitingCall,
                modifier = modifier,
            )
        }
    } else {
        val permissionManagerState = rememberPermissionManagerState(PermissionType.WriteExternalStorage)

        CardCornerButton(

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit cf4a2a4

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit d821fa2

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 92a6ac1

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch from 92a6ac1 to bc0d893 Compare April 16, 2026 09:03
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit bc0d893

Comment thread app/src/main/java/com/infomaniak/swisstransfer/ui/components/FileItemList.kt Outdated
@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch 2 times, most recently from 222b3a3 to 27702fb Compare April 16, 2026 14:09
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 27702fb

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch from 27702fb to 91662ff Compare April 17, 2026 08:05
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 91662ff

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch from 91662ff to 34ac7c7 Compare April 17, 2026 08:18
@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit 34ac7c7

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@github-actions
Copy link
Copy Markdown

Persistent review updated to latest commit f046119

@github-actions
Copy link
Copy Markdown

Failed to generate code suggestions for PR

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch 4 times, most recently from 5e193da to 5317f1d Compare April 23, 2026 12:52
@NicolasBourdin88 NicolasBourdin88 requested a review from LunarX April 23, 2026 12:52
@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch from 5317f1d to 263f941 Compare April 23, 2026 13:17
@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch 2 times, most recently from 9c54fed to 31c7576 Compare April 27, 2026 12:15
@github-actions
Copy link
Copy Markdown

This PR/issue depends on:

@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch 3 times, most recently from 44e137b to 2bfb417 Compare April 28, 2026 07:42
@NicolasBourdin88 NicolasBourdin88 force-pushed the feat/notification-permission-modal branch from 2bfb417 to d73576b Compare April 28, 2026 07:55
@sonarqubecloud
Copy link
Copy Markdown

@NicolasBourdin88 NicolasBourdin88 merged commit 96adda6 into main Apr 28, 2026
7 checks passed
@NicolasBourdin88 NicolasBourdin88 deleted the feat/notification-permission-modal branch April 28, 2026 08:00
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