feat: 动态管理设置页面导航项 #163
feat: 动态管理设置页面导航项 #163
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a TypeError in qfluentwidgets by implementing dynamic navigation item management in the settings window. When a settings page is unloaded to free memory, its navigation item is now removed from the sidebar, and when the page is recreated, the navigation item is automatically restored.
Changes:
- Added
_restore_navigation_item_if_neededmethod to restore navigation items when pages are recreated - Modified
_unload_settings_pageto remove navigation items and clear references when unloading pages - Updated
_on_stacked_widget_changedto call the restoration method when pages are recreated
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| interface_to_nav_item = { | ||
| "basicSettingsInterface": "basic_settings_item", | ||
| "listManagementInterface": "list_management_item", | ||
| "extractionSettingsInterface": "extraction_settings_item", | ||
| "floatingWindowManagementInterface": "floating_window_management_item", | ||
| "notificationSettingsInterface": "notification_settings_item", | ||
| "safetySettingsInterface": "safety_settings_item", | ||
| "courseSettingsInterface": "course_settings_item", | ||
| "voiceSettingsInterface": "voice_settings_item", | ||
| "historyInterface": "history_item", | ||
| "moreSettingsInterface": "more_settings_item", | ||
| "updateInterface": "update_item", | ||
| "aboutInterface": "about_item", | ||
| } |
There was a problem hiding this comment.
This interface-to-navigation-item mapping duplicates information from the nav_item_configs dictionary in _restore_navigation_item_if_needed (lines 546-619). The same mapping data is maintained in three separate places: initNavigation, _restore_navigation_item_if_needed, and here. This violates the DRY principle and makes the code harder to maintain. Consider extracting this mapping to a shared class-level constant.
| nav_item_configs = { | ||
| "basicSettingsInterface": { | ||
| "item_attr": "basic_settings_item", | ||
| "icon": "ic_fluent_wrench_settings_20_filled", | ||
| "text_module": "basic_settings", | ||
| "setting_key": "base_settings", | ||
| }, | ||
| "listManagementInterface": { | ||
| "item_attr": "list_management_item", | ||
| "icon": "ic_fluent_list_20_filled", | ||
| "text_module": "list_management", | ||
| "setting_key": "name_management", | ||
| }, | ||
| "extractionSettingsInterface": { | ||
| "item_attr": "extraction_settings_item", | ||
| "icon": "ic_fluent_archive_20_filled", | ||
| "text_module": "extraction_settings", | ||
| "setting_key": "draw_settings", | ||
| }, | ||
| "floatingWindowManagementInterface": { | ||
| "item_attr": "floating_window_management_item", | ||
| "icon": "ic_fluent_window_apps_20_filled", | ||
| "text_module": "floating_window_management", | ||
| "setting_key": "floating_window_management", | ||
| }, | ||
| "notificationSettingsInterface": { | ||
| "item_attr": "notification_settings_item", | ||
| "icon": "ic_fluent_comment_note_20_filled", | ||
| "text_module": "notification_settings", | ||
| "setting_key": "notification_service", | ||
| }, | ||
| "safetySettingsInterface": { | ||
| "item_attr": "safety_settings_item", | ||
| "icon": "ic_fluent_shield_20_filled", | ||
| "text_module": "safety_settings", | ||
| "setting_key": "security_settings", | ||
| }, | ||
| "courseSettingsInterface": { | ||
| "item_attr": "course_settings_item", | ||
| "icon": "ic_fluent_calendar_ltr_20_filled", | ||
| "text_module": "linkage_settings", | ||
| "setting_key": "linkage_settings", | ||
| }, | ||
| "voiceSettingsInterface": { | ||
| "item_attr": "voice_settings_item", | ||
| "icon": "ic_fluent_person_voice_20_filled", | ||
| "text_module": "voice_settings", | ||
| "setting_key": "voice_settings", | ||
| }, | ||
| "historyInterface": { | ||
| "item_attr": "history_item", | ||
| "icon": "ic_fluent_history_20_filled", | ||
| "text_module": "history", | ||
| "setting_key": "settings_history", | ||
| }, | ||
| "moreSettingsInterface": { | ||
| "item_attr": "more_settings_item", | ||
| "icon": "ic_fluent_more_horizontal_20_filled", | ||
| "text_module": "more_settings", | ||
| "setting_key": "more_settings", | ||
| }, | ||
| "updateInterface": { | ||
| "item_attr": "update_item", | ||
| "icon": "ic_fluent_arrow_sync_20_filled", | ||
| "text_module": "update", | ||
| "setting_key": None, # 固定显示 | ||
| }, | ||
| "aboutInterface": { | ||
| "item_attr": "about_item", | ||
| "icon": "ic_fluent_info_20_filled", | ||
| "text_module": "about", | ||
| "setting_key": None, # 固定显示 | ||
| }, | ||
| } |
There was a problem hiding this comment.
The navigation item configuration dictionary here duplicates data that already exists in the initNavigation method (lines 958-1039). This creates a maintenance burden as changes to navigation items would need to be updated in multiple places. Consider extracting this configuration to a class-level constant or shared data structure that can be used by both initNavigation, _restore_navigation_item_if_needed, and _unload_settings_page methods.
Fixes SECRANDOM-34. The issue was that: Navigation item attributes persist after their associated page widgets are unloaded, causing a TypeError when qfluentwidgets tries to animate a deleted widget.
_restore_navigation_item_if_needed方法用于在页面重新创建时恢复导航项。_unload_page方法，使其在卸载页面时同时移除对应的导航项并清除引用。This fix was generated by Seer in Sentry, triggered by wsxyt. 👁️ Run ID: 558735
Not quite right? Click here to continue debugging with Seer.