Skip to content

fix(platform): move out notification provider, to disable modal when not authed#2228

Merged
synoet merged 2 commits intomainfrom
synoet/move-out-notif-provider
Mar 26, 2026
Merged

fix(platform): move out notification provider, to disable modal when not authed#2228
synoet merged 2 commits intomainfrom
synoet/move-out-notif-provider

Conversation

@synoet
Copy link
Copy Markdown
Contributor

@synoet synoet commented Mar 26, 2026

  • fix(platform): move out notification provider to have access to user authentication check
  • remove

@synoet synoet requested a review from a team as a code owner March 26, 2026 21:29
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c2bb82c1-184e-4428-9ce2-5d9b1ea8a025

📥 Commits

Reviewing files that changed from the base of the PR and between b92a495 and 318a392.

📒 Files selected for processing (4)
  • js/app/packages/app/component/Root.tsx
  • js/app/packages/notifications/components/BrowserNotificationModal.tsx
  • js/app/packages/notifications/components/PlatformNotificationProvider.tsx
  • js/app/packages/tauri/src/TauriProvider.tsx

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Browser notification modal now only displays when users are authenticated, preventing notification prompts in non-authenticated states.
  • Improvements

    • Reorganized notification provider hierarchy to streamline integration with the core application framework.

Walkthrough

Reorganized notification provider integration by moving PlatformNotificationProvider to the Root component's outer provider hierarchy. Added authentication gating to BrowserNotificationModal display logic. Removed notification infrastructure from non-Tauri fallback path in TauriProvider.

Changes

Cohort / File(s) Summary
Provider Integration
js/app/packages/app/component/Root.tsx
Moved PlatformNotificationProvider to outer provider hierarchy, wrapping QuerySyncProviderWithUserId and all downstream providers including routing and toast region.
Notification Modal Enhancement
js/app/packages/notifications/components/BrowserNotificationModal.tsx, js/app/packages/notifications/components/PlatformNotificationProvider.tsx
Added BrowserNotificationModal component within notification provider; implemented authentication check to prevent modal display for unauthenticated users.
Tauri Provider Cleanup
js/app/packages/tauri/src/TauriProvider.tsx
Removed PlatformNotificationProvider and BrowserNotificationModal wrapping from non-Tauri fallback branch; now returns children directly.
🚥 Pre-merge checks | ✅ 1 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning The title exceeds the 72-character limit at 79 characters and does not follow conventional commits format strictly with a concise message. Shorten the title to under 72 characters, for example: 'fix: move notification provider out for auth check' (53 characters).
✅ Passed checks (1 passed)
Check name Status Explanation
Description check ✅ Passed The description partially relates to the changeset, mentioning the notification provider move and authentication check, which align with the actual changes made.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@synoet synoet changed the title synoet/move out notif provider fix(platform): move out notification provider, to disable modal when not authed Mar 26, 2026
@github-actions
Copy link
Copy Markdown

@synoet synoet requested a review from a team March 26, 2026 21:40
@synoet synoet merged commit c8a9fbc into main Mar 26, 2026
23 checks passed
@synoet synoet deleted the synoet/move-out-notif-provider branch March 26, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants