Skip to content
Snippets Groups Projects

Notification unification / WebPush i18n.

Merged lain requested to merge notification-unification-experiments into develop

This is a working implementation of localized and unified notifications.

  1. vuei18n won't work unless Vue is also imported.
  2. Trying to reuse the recently made on-demand loading of messages leads to webpack errors. I think the translation files have to be actually cached by the service worker / be in the manifest to make this work. Not sure though.
Edited by lain

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Maintainer

    Yeah translating the needed strings and sending them over to service worker would be a good idea. Unfortunately i have no idea how to implement that in a non-intrusive and maintainable way.

  • oh, you mean, just pulling in the two strings we need? mhh...

  • lain added 12 commits

    added 12 commits

    • 98819ae3...48365819 - 10 commits from branch develop
    • 33c1adbe - Merge branch 'develop' of git.pleroma.social:pleroma/pleroma-fe into...
    • 9bfb3754 - ServiceWorker: Use loader to only notification messages.

    Compare with previous version

  • lain unmarked as a Work In Progress

    unmarked as a Work In Progress

  • I fixed the webpack error by adding the hash to the chunk name, but it didn't make it actually possible to load the messages on demand, guess the service worker environment just doesn't allow dynamic imports like that.

    Either way, I solved this now by writing a small webpack loader that will only pull in the notification part of the files when used, so the translations can be pulled into the service worker at compile time without bloating it up so immensely.

  • lain changed the description

    changed the description

  • lain added 1 commit

    added 1 commit

    Compare with previous version

  • lain changed title from Notification unification experiments / WebPush i18n. to Notification unification / WebPush i18n.

    changed title from Notification unification experiments / WebPush i18n. to Notification unification / WebPush i18n.

  • Shpuld Shpludson
  • 1 1 /* eslint-env serviceworker */
    2 /* eslint-disable import/no-webpack-loader-syntax */
    2 3
    3 4 import localForage from 'localforage'
    5 import { parseNotification } from './services/entity_normalizer/entity_normalizer.service.js'
    6 import { prepareNotificationObject } from './services/notification_utils/notification_utils.js'
    7 import Vue from 'vue'
    8 import VueI18n from 'vue-i18n'
    9
    10 const messages = {
    11 ar: require('./lib/notification-i18n-loader.js!./i18n/ar.json'),
    • I see a lot of repetition in all these, not sure if worth replacing the prefix with a variable?

    • i wonder if a variable works as this is not dynamic loading but compile time.

    • I tried this, and it seems to work, but webpack does something different if you prefix with a variable:

      In this output, ar.json is loaded with a prefix.

          [./src sync recursive ^.*!\.\/i18n\/ar\.json$] ./src sync ^.*!\.\/i18n\/ar\.json$ 160 bytes {0} [built]
          [./src/lib/notification-i18n-loader.js!./src/i18n/ca.json] 293 bytes {0} [built]

      I'm a bit scared that this might lead to some issues down the road because it involves more compile time magic by webpack.

      What about extracting these messages to their own module too so at least this doesn't dominate the service worker code?

    • yeah if it creates extra stuff for the webpack compiler it's probably not worth the trouble like that. keeping it in it's own module is a good idea

      anyone adding a new language will not try to look into sw.js to add a line

    • lain changed this line in version 4 of the diff

      changed this line in version 4 of the diff

    • Please register or sign in to reply
  • Shpuld Shpludson
  • mentioned in issue pleroma#465 (closed)

  • mentioned in issue pleroma#464 (closed)

  • lain added 1 commit

    added 1 commit

    • 14540e2a - Service Worker: Extract messages to own module.

    Compare with previous version

  • Extracted the messages to another module.

  • Any more comments from you, @hj ?

  • ping once more @hj

  • 29 const state = await localForage.getItem('vuex-lz')
    30 const locale = state.config.interfaceLanguage || 'en'
    31 i18n.locale = locale
    32 }
    33
    34 const maybeShowNotification = async (event) => {
    35 const enabled = await isEnabled()
    36 const activeClients = await getWindowClients()
    37 await setLocale()
    38 if (enabled && (activeClients.length === 0)) {
    39 const data = event.data.json()
    40
    41 const url = `${self.registration.scope}api/v1/notifications/${data.notification_id}`
    42 const notification = await fetch(url, { headers: { Authorization: 'Bearer ' + data.access_token } })
    43 let nj = await notification.json()
    44 nj = parseNotification(nj)
  • Maintainer

    overall lgtm. don't really like async/await because it doesn't really make code more readable, but that's just my preference

    Edited by HJ
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading