[Research]: Build notification modal component #1935
Conversation
This research document captures the investigation into creating an alternative notification modal system for Teams for Linux to address cross-platform reliability issues with OS-level notifications. Key findings: - Current OS notifications unreliable on Linux (daemon freezing, inconsistencies) - Existing npm libraries too old (5-9 years) and incompatible with modern Electron - React toast libraries incompatible due to Electron wrapper architecture - Custom BrowserWindow implementation recommended following IncomingCallToast pattern Decisions documented: - UI Design: Follow Microsoft Teams design language - Actions: Start simple (View, Dismiss), iterate based on feedback - Shortcuts: None initially - Sounds: Reuse existing notification sounds - Tray: No integration needed - DND: Phase 2 feature linked to existing status detection Implementation approach: - Two-component system: NotificationToast (popups) + NotificationCenter (history) - IndexedDB for persistent storage (last 100 notifications, 7-day auto-cleanup) - Vanilla JavaScript/HTML/CSS (no React dependency) - Security-first: contextIsolation enabled, IPC validation - Timeline: 2-3 weeks for MVP Document includes: - Comprehensive library evaluation - Architectural constraints analysis - Complete implementation plan with phases - Risk assessment and mitigation strategies - Alternatives considered and rejected - Migration/rollout strategy (opt-in → beta → stable) Related: Investigation for custom notification modal alternative
Summary of ChangesHello @IsmaelMartinez, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a comprehensive research document outlining the design and implementation plan for a custom notification modal system within Teams for Linux. The initiative aims to overcome the limitations and unreliability of existing OS-level notifications, especially on Linux, by proposing a robust, in-app solution. The document details architectural constraints, evaluates alternative solutions, and provides a clear roadmap for building a secure, consistent, and feature-rich notification experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive and well-written research document for a new custom notification system. The document is thorough, covering all aspects from problem statement to implementation and rollout. The accompanying update to the research index file is also correct. I've added a few minor suggestions to the research document to improve the clarity and correctness of some code examples.
docs-site/docs/development/research/custom-notification-system-research.md
Outdated
Show resolved
Hide resolved
docs-site/docs/development/research/custom-notification-system-research.md
Outdated
Show resolved
Hide resolved
docs-site/docs/development/research/custom-notification-system-research.md
Outdated
Show resolved
Hide resolved
Fixes for custom-notification-system-research.md: 1. MDX Compilation Error (line 704): - Changed `<50ms` to `<50ms` to escape HTML entity - Prevents MDX from interpreting as invalid JSX tag 2. NotificationStore Class Issues: - Added EventEmitter import and extension - Added super() call in constructor - Added missing getUnreadCount() method used in IPC handler example 3. CSS Syntax Error (line 454): - Fixed invalid font-size property - Changed to valid CSS comment format 4. NotificationCenter Class: - Added missing updateBadge() method called in IPC handler example All code examples now compile correctly with Docusaurus MDX parser and match the implementation patterns used in the document.
The IncomingCallToast reference was linking to a source code directory (../../incomingCallToast/) which doesn't exist in the documentation site, causing Docusaurus build to fail. Changed from: - **[IncomingCallToast Implementation](../../incomingCallToast/)** To: - **IncomingCallToast Implementation** (`app/incomingCallToast/`) This clarifies that we're referencing source code patterns, not documentation, and resolves the Docusaurus broken link error.
Major updates to simplify MVP and clarify this is an opt-in alternative: Formatting & Clarity: - Fix "What We CAN Do" and "What We CANNOT Do" sections as proper lists - Fix Security Considerations section (6.3) as proper list - Update #1921 references to indicate closed/fixed issue Simplified MVP Approach: - Remove IndexedDB persistence from MVP (move to Phase 2) - Use in-memory notification storage for simplicity - Remove toast queue management from MVP (Phase 2 feature) - Remove maxVisible config option (add if users request it) - Timeline reduced from 3 weeks to 2 weeks Configuration Redesign: - Unified notification method: "web" | "electron" | "custom" - Group custom notification options into customNotification object - Align with existing notificationMethod pattern for consistency - Default remains "web" - no breaking changes - Explicit opt-in via notificationMethod: "custom" - Document backward compatibility strategy Rollout Strategy Changes: - This is an ALTERNATIVE, not a replacement - Keep all three notification methods fully supported - No deprecation of existing web/electron methods - Users choose based on their needs - Opt-in only - default unchanged Future Enhancements Updated: - Move storage/persistence to Phase 2 - Add toast queue management to Phase 2 - Add incoming call command integration to Phase 3 - Emphasize starting simple, iterate based on feedback - Only add complexity if users need it Success Criteria Updated: - Remove "persist across restarts" requirement - Change to "session-based" notification history - Focus on core functionality working reliably Conclusion Revised: - Emphasize this as third option for users with OS notification issues - Clarify no disruption to existing users - Highlight simplified MVP approach (2 weeks, basic features) - Key principle: Start simple, iterate based on actual user feedback
IsmaelMartinez
changed the title
…odal-investigation-011CUvMDXh3Bp77JUyexrp85
Major simplifications based on "start simple" principle: MVP Scope Changes: - Remove notification center entirely from MVP - Remove notification history/storage (no NotificationStore) - Remove all action buttons (View, Dismiss, etc.) - Remove toast queue management - Focus on single feature: show toast and auto-dismiss Architecture Updates: - Phase 1: Toast notifications only - Phase 2+: Everything else (center, history, actions, etc.) - Timeline reduced from 2-3 weeks to ~1 week Component Design: - Keep: NotificationToast only - Remove: NotificationCenter and NotificationStore from MVP - Simplified data model (just id, timestamp, title, body, icon) Configuration: - Remove showHistory option (not needed for MVP) - Simplified customNotification config object File Structure: - Flat structure (no center/ or store/ subdirectories) - Just 4 files: index.js, NotificationToast.js, .html, .preload.js IPC: - Single channel: 'notification-show-toast' - No complex handlers, just basic validation Future Enhancements: - Clearly separated all deferred features into Phase 2+ - Emphasized "add only if users request" philosophy This ultra-minimal MVP validates core functionality with minimal risk, development time, and maintenance burden.
…UvMDXh3Bp77JUyexrp85 Resolved conflict in docs-site/docs/development/research/README.md - Kept both sections: Architecture & Refactoring (from main) and Notification System Research (from branch) - Updated notification system timeline to reflect ultra-minimal MVP (~1 week)
…odal-investigation-011CUvMDXh3Bp77JUyexrp85
📦 PR Build Artifacts✅ Build successful! Download artifacts:
|
Changes: - Remove deprecated webNotification/electronNotification placeholders - Just extend existing notificationMethod with "custom" choice - Simplify customNotification config (remove "enabled" field) - Use notificationMethod value to determine if custom is active - Add note about ongoing config reorganization project This aligns with the project's existing config management strategy and avoids introducing deprecated options that will be migrated by the ongoing config reorganization work.
…odal-investigation-011CUvMDXh3Bp77JUyexrp85
|
|
blocked by #1935 |
This research document captures the investigation into creating an alternative notification modal system for Teams for Linux to address cross-platform reliability issues with OS-level notifications.
Key findings:
Decisions documented:
Implementation approach:
Document includes:
Related: Investigation for custom notification modal alternative