Skip to content

Hebrew translation + notification action buttons, opt-in prompt, Safari/iOS support#64

Closed
ClickAndGoScript wants to merge 18 commits into
NodeBB:mainfrom
ClickAndGoScript:main
Closed

Hebrew translation + notification action buttons, opt-in prompt, Safari/iOS support#64
ClickAndGoScript wants to merge 18 commits into
NodeBB:mainfrom
ClickAndGoScript:main

Conversation

@ClickAndGoScript

@ClickAndGoScript ClickAndGoScript commented Apr 27, 2026

Copy link
Copy Markdown

This PR started as Hebrew (he) translation support and grew while I was running the plugin on my own forum. What it contains now:

  • Hebrew translation, plus the missing zh-CN strings
  • "Mark as read" / "View notifications" action buttons on push notifications
  • An optional banner suggesting users enable push notifications — off by default, with an ACP setting for how many page visits before it shows
  • Service worker registration on Safari/iOS (16.4+), where core skips it, plus fixes to the settings page so it doesn't hang when no SW is registered
  • All remaining hardcoded strings (ACP page, admin menu item, test notification) moved to the language files

Known limitation: on Safari/iOS, older notifications with the same tag aren't cleared when a new one arrives, so users can end up with a few notifications with overlapping content. Details in the comments.

The commit history is messy — best to review the full diff.

Y-PLONI and others added 9 commits January 27, 2026 13:59
- הוספת nodebb-plugin-web-push-1.xml ל-.gitignore כדי למנוע מעקב אחר קבצי תצורה או יומנים שנוצרו על ידי הפלאגין, מה שמסייע בשמירה על ניקיון המאגר
- תיקון רווח קטן בטקסט ההתראה בקובץ השפה העברית כדי לשפר את הקריאות והעיצוב, ללא שינוי בתוכן הפונקציונלי
- שינוי שם התיקיה public/languages/he-IL ל-public/languages/he עבור קבצי השפה העברית (ללא שינוי בתוכן)
- הוספת בדיקת 'denied' לפני ניסיון הרשמה להתראות ב-settings.js
- טיפול ב-iOS Safari: שמירה על activation לפני בקשת הרשמה
- הוספת rollback אם הרשמה נכשלה אחרי יצירת subscription בדפדפן
- הוספת מפתחות תרגום חדשים: permission_denied ו-subscribe_failed באנגלית, עברית וסינית
Add feature detection for Service Worker and Push API to display user-friendly error messages when unsupported. Implement 5-second timeout for service worker registration to prevent UI hangs when SW fails to register, particularly common on iOS when the PWA isn't installed to Home Screen.
- plugin.json: הוספת הגדרת scripts שמפנה לקובץ main.js כדי לטעון את הסקריפט
- public/lib/main.js: מימוש רישום Service Worker ידני לדפדני Safari ו-iOS, מאחר ש-NodeBB מדלג על רישום אוטומטי בפלטפורמות אלו וגורם להשתקת דף ההגדרות
- הוספת לוג מפורט לכל אירועי מחזור החיים של ה-Service Worker כדי שכשלונות יופיעו ב-Web Inspector במקום להיעלם בשקט
- זיהוי Safari ו-iOS דרך User-Agent ותכונות maxTouchPoints, וביצוע רישום ידני רק בפלטפורמות אלו כדי לשמור על תאימות עם דפדפנים אחרים
- עדכון library.js: הוספת שדה mergeId לאובייקט הנתונים שנשלח בהתראה
- עדכון web-push.js: הוספת לוגיקה לסגירת התראות קיימות עם אותו mergeId לפני הצגת התראה חדשה
- פתרון בעיית תאימות ל-Safari שאינו תומך כראוי בתכונת 'tag' להחלפת התראות
Comment thread .gitignore Outdated
שליחת אופציית topic (גיבוב base64url של ה-tag, מוגבל ל-32 תווים) בכל
sendNotification. שירות הפוש של אפל ממפה את הכותרת Topic ל-apns-collapse-id,
כך שהתראה חדשה עם אותו topic מחליפה את הקודמת ברמת המערכת — גם בספארי
ו-iOS שמתעלמים מתכונת tag של ההתראה. בדפדפנים אחרים ה-Topic גם מחליף
הודעות שממתינות בתור כשהמכשיר לא מקוון.
- כפתור "סמן כנקרא" מסמן את ההתראה כנקראה ישירות מה-Service Worker
  באמצעות קריאת API (עובד גם כשאין חלון פתוח), כולל תמיכה בהתראות ממוזגות
- כפתור "צפה בהתראות" פותח את עמוד ההתראות (מתחשב בהתקנה בתת-נתיב)
- כותרות הכפתורים מתורגמות בצד השרת לפי שפת המשתמש (he, en-GB, zh-CN)
- הכפתורים מוצגים רק בדפדפנים התומכים ב-actions (לפי Notification.maxActions)
- לחיצה על גוף ההתראה ממשיכה לנווט ליעד כרגיל
- תיקון: מניעת שגיאה בלחיצה על התראה ללא כתובת יעד
@julianlam

Copy link
Copy Markdown
Member

Ah... A lot is going on here.

@ClickAndGoScript are you looking to just add the translations?

- אופציה חדשה בהגדרות הניהול: הצגת הצעת הרשמה למשתמשים מחוברים,
  כולל הגדרת מספר הביקורים בדף לפני ההצגה
- באנר פינתי שאינו חוסם את הדף ונסגר רק דרך הכפתורים שלו;
  ספירת הביקורים והדחייה נשמרות ב-localStorage לכל מכשיר
- הודעת הצלחה לאחר הרשמה, גם מהבאנר וגם מדף ההגדרות
- העברת כל המחרוזות הקשיחות לקבצי השפה (תבנית הניהול, פריט התפריט
  והתראת הבדיקה) בשלוש השפות הנתמכות
המנגנון נועד לגרום להתראה חדשה להחליף את הקודמת בספארי/iOS
(דרך מיפוי ל-apns-collapse-id), אך בפועל אינו עובד. ההחלפה
בכרום ובפיירפוקס ממשיכה לפעול דרך ה-tag שבהתראה עצמה,
ומנגנון ה-rescind אינו מושפע.
@ClickAndGoScript ClickAndGoScript changed the title i18n: add Hebrew translation for plugin terms Hebrew translation + notification action buttons, opt-in prompt, Safari/iOS support Jun 12, 2026
@ClickAndGoScript

Copy link
Copy Markdown
Author

@julianlam yeah, sorry about that :) It did start as just the translations, but I've been running the plugin on my own forum since then and kept pushing fixes and features to the same branch. The commit history ended up messy (experiments, reverts, merge commits), so it's best to review the full diff and ignore the individual commits. I've updated the PR title and description to reflect what's actually in here now.

What's in here besides the Hebrew translation:

  • Action buttons on push notifications: "Mark as read" and "View notifications". Mark as read works from the service worker itself, since there's usually no forum tab open when you tap a push notification.
  • An optional banner suggesting users enable push notifications, shown to logged-in users after X page loads. Off by default, configurable in the ACP, and dismissing it is remembered per device so it won't nag.
  • Service worker registration on Safari/iOS. Core skips SW registration on Safari, which predates iOS 16.4 where push is actually supported now, so the plugin registers it itself. Also fixed the settings page hanging forever on serviceWorker.ready when no SW is registered.
  • Moved the remaining hardcoded strings (ACP page, admin menu item, test notification) into the language files, so the test notification arrives translated too. Added the missing zh-CN strings while at it.

One known limitation I couldn't solve: on Safari/iOS, older notifications aren't cleared when a new one with the same tag arrives. To be clear, nothing arrives twice - every notification shows up exactly when it should. It's just that when notifications share a tag (e.g. consecutive chat messages that get merged), the new one is supposed to replace the old one, and on Safari the old ones stay, so the user can end up with a few notifications stacked up with overlapping content. I tried the Topic header (apns-collapse-id) route, it didn't work, so I removed that code. Still better than the current state, where Safari users get no notifications at all.

If you'd rather take just the translation and have the rest split into separate PRs, no problem, let me know.

- מודול storage במקום גישה ישירה ל-localStorage בבאנר ההרשמה,
  כולל fallback מובנה כשהאחסון חסום בדפדפן
- תבנית partials/web-push/prompt.tpl המרונדרת עם app.parseAndTranslate
  במקום בניית HTML כמחרוזת בתוך הקוד
- זיהוי ספארי לפי config.useragent.isSafari - אותו דגל שה-core בודק
  כשהוא מדלג על רישום ה-Service Worker - במקום regex ידני על ה-UA
@julianlam

Copy link
Copy Markdown
Member

Yes, the problem here is the commit history is messy so I would squash+merge but I prefer to rebase+merge.

Are you able to get claude (or whatever AI you use) to split this PR into four?

@ClickAndGoScript

Copy link
Copy Markdown
Author

Okay, I'll try to split it up.
Actually, I already did a more organized restoration of the commits, but since the PR was still open since I opened it previously for the translations, it just made more of a mess (in the PR, not in my code, so it wouldn't be too difficult for me to reopen it in an organized way).

@julianlam

Copy link
Copy Markdown
Member

It may be easiest for you to instruct the LLM to create new branches for each feature (translations, action buttons, opt-in prompt, Safari/iOS support)

Then it won't interfere with your existing branch that you use in your site.

@ClickAndGoScript

Copy link
Copy Markdown
Author

I don't think these additions are extensive enough to allocate a separate branch to each of them, but I might actually do everything in the dev branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants