-
Notifications
You must be signed in to change notification settings - Fork 102
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
multi: Implement OSDesktopNotifier for MacOS and some refactoring #2631
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,79 +35,120 @@ const NoteTypeMatch = 'match' | |
const NoteTypeBondPost = 'bondpost' | ||
const NoteTypeConnEvent = 'conn' | ||
|
||
type BrowserNtfnSettingLabel = { | ||
type DesktopNtfnSettingLabel = { | ||
[x: string]: string | ||
} | ||
|
||
type BrowserNtfnSetting = { | ||
export type DesktopNtfnSetting = { | ||
[x: string]: boolean | ||
} | ||
|
||
function browserNotificationsSettingsKey (): string { | ||
return `browser_notifications-${window.location.host}` | ||
function desktopNtfnSettingsKey (): string { | ||
return `desktop_notifications-${window.location.host}` | ||
} | ||
|
||
export const browserNtfnLabels: BrowserNtfnSettingLabel = { | ||
export const desktopNtfnLabels: DesktopNtfnSettingLabel = { | ||
[NoteTypeOrder]: intl.ID_BROWSER_NTFN_ORDERS, | ||
[NoteTypeMatch]: intl.ID_BROWSER_NTFN_MATCHES, | ||
[NoteTypeBondPost]: intl.ID_BROWSER_NTFN_BONDS, | ||
[NoteTypeConnEvent]: intl.ID_BROWSER_NTFN_CONNECTIONS | ||
} | ||
|
||
export const defaultBrowserNtfnSettings: BrowserNtfnSetting = { | ||
export const defaultDesktopNtfnSettings: DesktopNtfnSetting = { | ||
[NoteTypeOrder]: true, | ||
[NoteTypeMatch]: true, | ||
[NoteTypeBondPost]: true, | ||
[NoteTypeConnEvent]: true | ||
} | ||
|
||
let browserNtfnSettings: BrowserNtfnSetting | ||
let desktopNtfnSettings: DesktopNtfnSetting | ||
|
||
export function ntfnPermissionGranted () { | ||
return window.Notification.permission === 'granted' | ||
} | ||
// BrowserNotifier is a wrapper around the browser's notification API. | ||
class BrowserNotifier { | ||
static ntfnPermissionGranted (): boolean { | ||
return window.Notification.permission === 'granted' | ||
} | ||
|
||
static ntfnPermissionDenied (): boolean { | ||
return window.Notification.permission === 'denied' | ||
} | ||
|
||
export function ntfnPermissionDenied () { | ||
return window.Notification.permission === 'denied' | ||
static async requestNtfnPermission (): Promise<void> { | ||
if (!('Notification' in window)) { | ||
return | ||
} | ||
if (BrowserNotifier.ntfnPermissionGranted()) { | ||
BrowserNotifier.sendDesktopNotification(intl.prep(intl.ID_BROWSER_NTFN_ENABLED)) | ||
} else if (!BrowserNotifier.ntfnPermissionDenied()) { | ||
await Notification.requestPermission() | ||
BrowserNotifier.sendDesktopNotification(intl.prep(intl.ID_BROWSER_NTFN_ENABLED)) | ||
} | ||
} | ||
|
||
static sendDesktopNotification (title: string, body?: string) { | ||
if (!BrowserNotifier.ntfnPermissionGranted() || !desktopNtfnSettings.browserNtfnEnabled) return | ||
const ntfn = new window.Notification(title, { | ||
body: body, | ||
icon: '/img/softened-icon.png' | ||
}) | ||
return ntfn | ||
} | ||
} | ||
|
||
export async function requestNtfnPermission () { | ||
if (!('Notification' in window)) { | ||
return | ||
// OSDesktopNotifier manages OS desktop notifications via the same interface | ||
// as BrowserNotifier, but sends notifications using an underlying Go | ||
// notification library exposed to the webview. | ||
class OSDesktopNotifier { | ||
static ntfnPermissionGranted (): boolean { | ||
ukane-philemon marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return true | ||
} | ||
if (Notification.permission === 'granted') { | ||
showBrowserNtfn(intl.prep(intl.ID_BROWSER_NTFN_ENABLED)) | ||
} else if (Notification.permission !== 'denied') { | ||
await Notification.requestPermission() | ||
showBrowserNtfn(intl.prep(intl.ID_BROWSER_NTFN_ENABLED)) | ||
|
||
static ntfnPermissionDenied (): boolean { | ||
return false | ||
} | ||
|
||
static async requestNtfnPermission (): Promise<void> { | ||
OSDesktopNotifier.sendDesktopNotification(intl.prep(intl.ID_BROWSER_NTFN_ENABLED)) | ||
return Promise.resolve() | ||
} | ||
|
||
static async sendDesktopNotification (title: string, body?: string): Promise<void> { | ||
if (!desktopNtfnSettings.browserNtfnEnabled) return | ||
if (window.webkit) await window.webkit.messageHandlers.dexcHandler.postMessage(['sendOSNotification', title, body]) // See: client/cmd/dexc-desktop/app_darwin.go#L673-#L697. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It works correctly because we test for webview first. But it did seem a bit brittle indeed, refactored the check to not rely on |
||
else await (window as any).sendOSNotification(title, body) // this calls a function exported via webview.Bind() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead of window as any add the method to the sendOSNotification: (title: string, body?: string) => void There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This PR was superseded by #2599, these changes were rolled into it. |
||
} | ||
} | ||
|
||
export function showBrowserNtfn (title: string, body?: string) { | ||
if (window.Notification.permission !== 'granted') return | ||
const ntfn = new window.Notification(title, { | ||
body: body, | ||
icon: '/img/softened-icon.png' | ||
}) | ||
return ntfn | ||
// isWebview checks if we are running in webview or webkit (MacOS). | ||
function isWebview (): boolean { | ||
return window.isWebview !== undefined || window.webkit !== undefined // MacOS | ||
} | ||
|
||
export function browserNotify (note: CoreNote) { | ||
if (!browserNtfnSettings[note.type]) return | ||
showBrowserNtfn(note.subject, note.details) | ||
// determine whether we're running in a webview or in browser, and export | ||
// the appropriate notifier accordingly. | ||
export const Notifier = isWebview() ? OSDesktopNotifier : BrowserNotifier | ||
|
||
export const ntfnPermissionGranted = Notifier.ntfnPermissionGranted | ||
export const ntfnPermissionDenied = Notifier.ntfnPermissionDenied | ||
export const requestNtfnPermission = Notifier.requestNtfnPermission | ||
export const sendDesktopNotification = Notifier.sendDesktopNotification | ||
|
||
export function desktopNotify (note: CoreNote) { | ||
if (!desktopNtfnSettings[note.type]) return | ||
Notifier.sendDesktopNotification(note.subject, note.details) | ||
} | ||
|
||
export async function fetchBrowserNtfnSettings (): Promise<BrowserNtfnSetting> { | ||
if (browserNtfnSettings !== undefined) { | ||
return browserNtfnSettings | ||
export function fetchDesktopNtfnSettings (): DesktopNtfnSetting { | ||
if (desktopNtfnSettings !== undefined) { | ||
return desktopNtfnSettings | ||
} | ||
const k = browserNotificationsSettingsKey() | ||
browserNtfnSettings = (await State.fetchLocal(k) ?? {}) as BrowserNtfnSetting | ||
return browserNtfnSettings | ||
const k = desktopNtfnSettingsKey() | ||
desktopNtfnSettings = (State.fetchLocal(k) ?? {}) as DesktopNtfnSetting | ||
return desktopNtfnSettings | ||
} | ||
|
||
export async function updateNtfnSetting (noteType: string, enabled: boolean) { | ||
await fetchBrowserNtfnSettings() | ||
browserNtfnSettings[noteType] = enabled | ||
State.storeLocal(browserNotificationsSettingsKey(), browserNtfnSettings) | ||
fetchDesktopNtfnSettings() | ||
desktopNtfnSettings[noteType] = enabled | ||
State.storeLocal(desktopNtfnSettingsKey(), desktopNtfnSettings) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -175,25 +175,23 @@ export default class SettingsPage extends BasePage { | |
this.renderDesktopNtfnSettings() | ||
} | ||
|
||
async updateNtfnSetting (e: Event) { | ||
updateNtfnSetting (e: Event) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need for the |
||
const checkbox = e.target as HTMLInputElement | ||
const noteType = checkbox.getAttribute('name') | ||
if (noteType === null) return | ||
const enabled = checkbox.checked | ||
await ntfn.updateNtfnSetting(noteType, enabled) | ||
ntfn.updateNtfnSetting(noteType, enabled) | ||
} | ||
|
||
async getBrowserNtfnSettings (form: HTMLElement) { | ||
const loaded = app().loading(form) | ||
const permissions = await ntfn.fetchBrowserNtfnSettings() | ||
loaded() | ||
getBrowserNtfnSettings (): ntfn.DesktopNtfnSetting { | ||
const permissions = ntfn.fetchDesktopNtfnSettings() | ||
return permissions | ||
} | ||
|
||
async renderDesktopNtfnSettings () { | ||
const page = this.page | ||
const ntfnSettings = await this.getBrowserNtfnSettings(page.browserNotificationsForm) | ||
const labels = ntfn.browserNtfnLabels | ||
const ntfnSettings = this.getBrowserNtfnSettings() | ||
const labels = ntfn.desktopNtfnLabels | ||
const tmpl = page.browserNtfnCheckboxTemplate | ||
tmpl.removeAttribute('id') | ||
const container = page.browserNtfnCheckboxContainer | ||
|
@@ -219,7 +217,7 @@ export default class SettingsPage extends BasePage { | |
await ntfn.requestNtfnPermission() | ||
checkbox.checked = !ntfn.ntfnPermissionDenied() | ||
} | ||
await this.updateNtfnSetting(e) | ||
this.updateNtfnSetting(e) | ||
checkbox.dispatchEvent(new Event('change')) | ||
}) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code still manages the browser ntfn's - as well as the desktop toasts. Renaming these functions to suggest they handle desktop notifications only isn't warranted.