Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where clicking on the update log notification doesn't properly open the documentation page. The fix moves the notification creation logic to a centralized location and adds support for URL options in notifications.
Changes:
- Moved
InfoNotificationfunction from utils to service worker with enhanced URL handling capability - Added caching mechanism to store notification options (including URLs) for later retrieval
- Implemented click handler in GMApi to open URLs when notifications are clicked
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/pkg/utils/utils.ts | Removed the original InfoNotification function |
| src/app/service/service_worker/utils.ts | Added enhanced InfoNotification function with URL option support and caching |
| src/app/service/service_worker/synchronize.ts | Updated import to use new InfoNotification location |
| src/app/service/service_worker/subscribe.ts | Updated import to use new InfoNotification location |
| src/app/service/service_worker/index.ts | Updated import and added URL parameter when creating update notification |
| src/app/service/service_worker/gm_api/gm_api.ts | Added notification click handler to open URLs and clean up cache |
| src/manifest.json | Bumped version from 1.3.0.1400 to 1.3.0.1500 |
| package.json | Bumped version from 1.3.0-beta.3 to 1.3.0-beta.4 |
Comment on lines
+1338
to
+1341
| } else if (event === "close") { | ||
| // 删除缓存 | ||
| cacheInstance.del(`notification:${notificationId}:options`); | ||
| } |
There was a problem hiding this comment.
当通知被清除时,缓存也应该被删除。建议在第1336行 chrome.notifications.clear(notificationId) 之前也调用 cacheInstance.del(),以确保在所有情况下都能正确清理缓存。或者,可以将缓存删除逻辑提取为一个独立的函数来避免重复。
| export type NotificationOptionCache = { | ||
| url?: string; | ||
| }; | ||
|
|
There was a problem hiding this comment.
函数缺少文档注释。建议添加 JSDoc 注释说明该函数的用途、参数含义以及 options 参数的作用,特别是 URL 选项会如何影响通知的行为。
Suggested change
| /** | |
| * 显示基础通知，并可选地为通知绑定额外参数（如跳转 URL）。 | |
| * | |
| * 当提供 {@link NotificationOptionCache.url} 时，会将该选项与通知 ID 关联后 | |
| * 缓存到 {@link cacheInstance} 中，键名格式为 `notification:{notificationId}:options`， | |
| * 以便在 gm_api 中统一根据通知 ID 读取并处理（例如在点击通知时打开对应的 URL）。 | |
| * | |
| * @param title 通知标题。 | |
| * @param msg 通知内容文本。 | |
| * @param options 额外的通知选项，例如 `url` 字段用于指定后续处理时需要打开的页面地址。 | |
| */ |
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
1 participant
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.
概述 Descriptions
变更内容 Changes
截图 Screenshots