-
Notifications
You must be signed in to change notification settings - Fork 55
feat: add i18n support for notification clear button #1268
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
Conversation
1. Added translation handling for NotifyItemContent.qml in CMakeLists.txt 2. Replaced SettingActionButton with AnimationSettingButton for better UX 3. Added text label "Clear alone" to the close button for accessibility 4. Anchored close button to the right side for proper layout 5. Created translation files for 20+ languages including Chinese variants 6. Pre-filled Chinese translations (zh_CN, zh_HK, zh_TW) while leaving others unfinished This change enables internationalization support for the notification clear button, making the interface more user-friendly for global users. The button now has proper text labeling and improved positioning. feat: 为通知清除按钮添加国际化支持 1. 在 CMakeLists.txt 中添加对 NotifyItemContent.qml 的翻译处理 2. 将 SettingActionButton 替换为 AnimationSettingButton 以提升用户体验 3. 为关闭按钮添加"清除单个"文本标签以提高可访问性 4. 将关闭按钮锚定到右侧以实现正确布局 5. 创建包含中文变体在内的20多种语言翻译文件 6. 预填充中文翻译(简体、香港、台湾),其他语言留空待翻译 此更改使通知清除按钮支持国际化,为全球用户提供更友好的界面。按钮现在具有 适当的文本标签和改善的定位。 Pms: BUG-283587
Reviewer's GuideThis PR adds internationalization support for the notification clear button by updating the QML component for accessibility and UX, integrating translation handling in the build system, and scaffolding TS files for 20+ languages with prefilled Chinese variants. Class diagram for updated NotifyItemContent QML componentclassDiagram
class NotifyItemContent {
+Loader closeBtn
+AnimationSettingButton replaces SettingActionButton
+text: qsTr("Clear alone")
+anchors.right: parent.right
}
NotifyItemContent --> AnimationSettingButton
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- Remove the unused import org.deepin.ds.notificationcenter from NotifyItemContent.qml to keep dependencies minimal.
- Revisit the 'Clear alone' text for clarity—consider renaming it to something like 'Clear notification' or 'Dismiss this' to improve user understanding.
- Verify that switching to AnimationSettingButton preserves the original focus and hover behaviors of the close button across all notification scenarios.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Remove the unused import org.deepin.ds.notificationcenter from NotifyItemContent.qml to keep dependencies minimal.
- Revisit the 'Clear alone' text for clarity—consider renaming it to something like 'Clear notification' or 'Dismiss this' to improve user understanding.
- Verify that switching to AnimationSettingButton preserves the original focus and hover behaviors of the close button across all notification scenarios.
## Individual Comments
### Comment 1
<location> `panels/notification/plugin/NotifyItemContent.qml:70` </location>
<code_context>
id: closeBtn
objectName: "closeNotify-" + root.appName
icon.name: "clean-alone"
+ text: qsTr("Clear alone")
padding: 2
forcusBorderVisible: visualFocus
</code_context>
<issue_to_address>
**suggestion:** Review the phrasing of 'Clear alone' for clarity.
Consider using a more descriptive label, such as 'Clear this notification', to improve clarity unless 'Clear alone' is needed for consistency.
```suggestion
text: qsTr("Clear this notification")
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| anchors.right: parent.right | ||
| active: !(root.strongInteractive && root.actions.length > 0) && (root.closeVisible || closePlaceHolder.hovered || activeFocus) | ||
| sourceComponent: SettingActionButton { | ||
| sourceComponent: AnimationSettingButton { |
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.
AnimationSettingButton 应该可以移动到plugin中,
center依赖plugin,而不应该plugin依赖center,
这里的清除按钮也要改成这种么,
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.
是的
|
|
||
| install(TARGETS ds-notification-shared DESTINATION "${LIB_INSTALL_DIR}") | ||
| ds_install_package(PACKAGE org.deepin.ds.notification TARGET ds-notification) | ||
| ds_handle_package_translation(PACKAGE org.deepin.ds.notification QML_FILES ${CMAKE_CURRENT_SOURCE_DIR}/plugin/NotifyItemContent.qml) |
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.
这里获取所有的文件吧,免得之后要单个加需要翻译的文件,
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.
新资源需要加到 .tx/transifex.yaml 里
This change enables internationalization support for the notification clear button, making the interface more user-friendly for global users. The button now has proper text labeling and improved positioning.
feat: 为通知清除按钮添加国际化支持
此更改使通知清除按钮支持国际化,为全球用户提供更友好的界面。按钮现在具有
适当的文本标签和改善的定位。
Pms: BUG-283587
Summary by Sourcery
Add internationalization support for the notification clear button and enhance its layout and UX
New Features:
Enhancements:
Build: