macOS: tie wx notifications to main window and improve failure visibility#69
Closed
dominicletz wants to merge 3 commits intomainfrom
Closed
macOS: tie wx notifications to main window and improve failure visibility#69dominicletz wants to merge 3 commits intomainfrom
dominicletz wants to merge 3 commits intomainfrom
Conversation
- Call wxNotificationMessage:setParent/2 on macOS so Notification Center ties the toast to the app window (wx default main window can be wrong for BEAM). - Log when show/2 returns false with pointers to issue #38 and settings. - Add ELIXIR_DESKTOP_OS=macos for testable macOS branches; add OS.macos?/0. - Guard notification_new/show when NO_WX yields nil notification refs. Refs #38 Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
dominicletz
added
the
cursor-waiting
Cursor
|
@gemini review this PR |
cursor-automation
Bot
added
cursor-waiting-for-ci
Bind frame in show_notification cast (was undefined), recreate wx notification when a stored id maps to nil, no-op notification_close/1 for nil, fix doc typo, and drop duplicate :dismiss callback on non-Linux. Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
Igniter.Project.Module.module_name/2 is specced as (Igniter.t(), String.t()). Using the atom MainWindow broke Dialyzer (invalid call, no_return on igniter/1, and spurious unused_fun on private helpers). Use "MainWindow" consistently. Unblocks CI for #69 Co-authored-by: Dominic Letz <dominicletz@users.noreply.github.com>
dominicletz
added
the
cursor-waiting-for-human
Cursor
dominicletz
removed
the
cursor-waiting-for-ci
cursor-automation
Bot
added
cursor-ignore
dominicletz
removed
the
cursor-ignore
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This change improves macOS desktop notifications for Elixir Desktop by wiring
wxNotificationMessageto the samewxFrameas the Phoenix/LiveView window, treating afalsereturn fromshow/2as a first-class failure with actionable logging, and making macOS-specific branches testable from Linux CI viaELIXIR_DESKTOP_OS=macos.Refs #38
Root cause (why this is not a band-aid)
wxWidgets documents that notifications are associated with the top-level parent of the window passed to
setParent, or with the application’s default main window ifsetParentis never called. The BEAM process often has no meaningful “main window” from the OS’s perspective when running underbeam.smpduring development, so Notification Center can accept the API call yet deliver nothing visible—or associate the toast with the wrong identity. Passing the actualDesktop.Windowframe as the parent aligns the notification with the same UI surface the user sees, which matches how macOS expects user notifications to be scoped.This does not replace the separate packaging concern (matching
Info.plistin the.appbundle with metadata onbeam.smpfor production); it addresses the in-process wx association and silent failure parts of the same issue.Changes
Desktop.Fallback.notification_new/3optional parent: on macOS, call:wxNotificationMessage.setParent/2when available.Desktop.Windowpassesframewhen creating/reusing notifications; module doc notes macOS troubleshooting.notification_show/4logs a warning whenshow/2returnsfalse, with a pointer to issue Notifications not showing on OSX #38 and System Settings.Desktop.OS:ELIXIR_DESKTOP_OS=macosoverride for tests; publicmacos?/0.notification_new/notification_showwhenNO_WXleaves refs nil (avoids crashingconnecton nil).Tests
test/os_test.exs—ELIXIR_DESKTOP_OS=macosforcesDesktop.OS.type/0 == MacOS.test/fallback_notification_test.exs—notification_new/3withNO_WX=1does not raise (regression guard for nil notification path).CI should run
mix testandmix linton the merge branch.