From 724ca3748de5a3aefc98999118a5310ac4b820c1 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:12:44 +0000 Subject: [PATCH 1/8] refactor(ui): improve settings dialog UX flow --- .../ui/dialogs/settings_dialog.py | 138 +++++++-- .../ui/dialogs/settings_tabs/advanced.py | 202 ++++++------- .../ui/dialogs/settings_tabs/ai.py | 166 ++++++----- .../ui/dialogs/settings_tabs/audio.py | 132 +++++---- .../ui/dialogs/settings_tabs/data_sources.py | 266 +++++++++++------- .../ui/dialogs/settings_tabs/display.py | 223 ++++++++------- .../ui/dialogs/settings_tabs/general.py | 111 +++++--- .../ui/dialogs/settings_tabs/notifications.py | 221 ++++++++------- .../ui/dialogs/settings_tabs/updates.py | 82 ++++-- tests/test_settings_dialog_audio_events.py | 4 +- tests/test_settings_dialog_ux_metadata.py | 54 ++++ 11 files changed, 976 insertions(+), 623 deletions(-) create mode 100644 tests/test_settings_dialog_ux_metadata.py diff --git a/src/accessiweather/ui/dialogs/settings_dialog.py b/src/accessiweather/ui/dialogs/settings_dialog.py index 1e58f7ff..ae345b4d 100644 --- a/src/accessiweather/ui/dialogs/settings_dialog.py +++ b/src/accessiweather/ui/dialogs/settings_dialog.py @@ -5,6 +5,7 @@ import json import logging import webbrowser +from contextlib import suppress from pathlib import Path from typing import TYPE_CHECKING @@ -99,12 +100,23 @@ def _on_ok(self, event): class SettingsDialogSimple(wx.Dialog): """Comprehensive settings dialog — thin coordinator over per-tab modules.""" + _TAB_DEFINITIONS = [ + ("general", "General"), + ("display", "Display"), + ("notifications", "Alerts"), + ("audio", "Audio"), + ("data_sources", "Data Sources"), + ("ai", "AI"), + ("updates", "Updates"), + ("advanced", "Advanced"), + ] + def __init__(self, parent, app: AccessiWeatherApp): """Initialize the settings dialog.""" super().__init__( parent, title="Settings", - size=(600, 550), + size=(760, 640), style=wx.DEFAULT_DIALOG_STYLE | wx.RESIZE_BORDER, ) self.app = app @@ -118,6 +130,11 @@ def __init__(self, parent, app: AccessiWeatherApp): self._load_settings() self._setup_accessibility() + @classmethod + def get_tab_definitions(cls) -> list[tuple[str, str]]: + """Return notebook tab keys and visible labels in display order.""" + return list(cls._TAB_DEFINITIONS) + # ------------------------------------------------------------------ # Delegation helpers for backward compatibility # ------------------------------------------------------------------ @@ -140,6 +157,65 @@ def _build_default_source_settings_states() -> dict: # UI creation # ------------------------------------------------------------------ + @staticmethod + def _wrap_static_text(control: wx.Window, width: int = 620) -> wx.Window: + """Wrap static text where supported to keep copy readable.""" + if hasattr(control, "Wrap"): + with suppress(Exception): + control.Wrap(width) + return control + + def add_help_text( + self, + parent: wx.Window, + parent_sizer: wx.Sizer, + text: str, + *, + left: int = 10, + bottom: int = 8, + ) -> wx.StaticText: + """Add wrapped helper text to a sizer.""" + control = wx.StaticText(parent, label=text) + self._wrap_static_text(control) + parent_sizer.Add(control, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, left) + return control + + def add_labeled_row( + self, + parent: wx.Window, + parent_sizer: wx.Sizer, + label: str, + control: wx.Window, + *, + expand_control: bool = False, + bottom: int = 8, + ) -> wx.BoxSizer: + """Add a consistent label/control row to a sizer.""" + row = wx.BoxSizer(wx.HORIZONTAL) + row.Add( + wx.StaticText(parent, label=label), + 0, + wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + 10, + ) + row.Add(control, 1 if expand_control else 0, wx.EXPAND if expand_control else 0) + parent_sizer.Add(row, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, bottom) + return row + + def create_section( + self, + parent: wx.Window, + parent_sizer: wx.Sizer, + title: str, + description: str | None = None, + ) -> wx.StaticBoxSizer: + """Create a titled settings section with optional helper copy.""" + section = wx.StaticBoxSizer(wx.VERTICAL, parent, title) + if description: + self.add_help_text(parent, section, description) + parent_sizer.Add(section, 0, wx.EXPAND | wx.ALL, 5) + return section + def _create_ui(self): """Create the dialog UI using per-tab modules.""" from .settings_tabs import ( @@ -154,32 +230,54 @@ def _create_ui(self): ) main_sizer = wx.BoxSizer(wx.VERTICAL) + intro = wx.StaticText( + self, + label=( + "Review preferences by category. Changes are saved when you choose Save Settings." + ), + ) + self._wrap_static_text(intro) + main_sizer.Add(intro, 0, wx.LEFT | wx.RIGHT | wx.TOP | wx.EXPAND, 10) + sub_intro = wx.StaticText( + self, + label=( + "Everyday preferences appear first. Maintenance, backup, and reset " + "tools are grouped on the Advanced tab." + ), + ) + self._wrap_static_text(sub_intro) + main_sizer.Add(sub_intro, 0, wx.LEFT | wx.RIGHT | wx.TOP | wx.EXPAND, 10) self.notebook = wx.Notebook(self) + tab_classes = { + "general": GeneralTab, + "display": DisplayTab, + "notifications": NotificationsTab, + "audio": AudioTab, + "data_sources": DataSourcesTab, + "ai": AITab, + "updates": UpdatesTab, + "advanced": AdvancedTab, + } - self._tab_objects = [ - GeneralTab(self), - DisplayTab(self), - DataSourcesTab(self), - NotificationsTab(self), - AudioTab(self), - UpdatesTab(self), - AITab(self), - AdvancedTab(self), - ] - # Keep named references for methods that need specific tabs - self._audio_tab = self._tab_objects[4] - self._display_tab = self._tab_objects[1] - self._data_sources_tab = self._tab_objects[2] + self._tab_objects = [] + self._tab_objects_by_key = {} + for tab_key, page_label in self.get_tab_definitions(): + tab = tab_classes[tab_key](self) + self._tab_objects.append(tab) + self._tab_objects_by_key[tab_key] = tab + tab.create(page_label=page_label) - for tab in self._tab_objects: - tab.create() + # Keep named references for methods that need specific tabs + self._audio_tab = self._tab_objects_by_key["audio"] + self._display_tab = self._tab_objects_by_key["display"] + self._data_sources_tab = self._tab_objects_by_key["data_sources"] main_sizer.Add(self.notebook, 1, wx.EXPAND | wx.ALL, 10) button_sizer = wx.BoxSizer(wx.HORIZONTAL) button_sizer.AddStretchSpacer() - ok_btn = wx.Button(self, wx.ID_OK, "OK") + ok_btn = wx.Button(self, wx.ID_OK, "Save Settings") ok_btn.Bind(wx.EVT_BUTTON, self._on_ok) cancel_btn = wx.Button(self, wx.ID_CANCEL, "Cancel") @@ -188,6 +286,10 @@ def _create_ui(self): main_sizer.Add(button_sizer, 0, wx.EXPAND | wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) self.SetSizer(main_sizer) + self.SetMinSize((700, 580)) + self.SetAffirmativeId(wx.ID_OK) + self.SetEscapeId(wx.ID_CANCEL) + ok_btn.SetDefault() # ------------------------------------------------------------------ # Load / Save / Accessibility (delegate to tab objects) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/advanced.py b/src/accessiweather/ui/dialogs/settings_tabs/advanced.py index 32086f98..54e85683 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/advanced.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/advanced.py @@ -16,127 +16,137 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "Advanced"): """Build the Advanced tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - # System options - controls["minimize_tray"] = wx.CheckBox( - panel, label="Minimize to notification area when closing" + self.dialog.add_help_text( + panel, + sizer, + "Advanced settings include startup behavior, backup tools, and maintenance actions that you may not need every day.", + left=5, ) - controls["minimize_tray"].Bind(wx.EVT_CHECKBOX, self.dialog._on_minimize_tray_changed) - sizer.Add(controls["minimize_tray"], 0, wx.ALL, 5) + startup_section = self.dialog.create_section( + panel, + sizer, + "Startup and window behavior", + "These options change how AccessiWeather launches and where it goes when you close or minimize it.", + ) + controls["minimize_tray"] = wx.CheckBox( + panel, + label="Minimize to the notification area when closing", + ) + controls["minimize_tray"].Bind( + wx.EVT_CHECKBOX, + self.dialog._on_minimize_tray_changed, + ) + startup_section.Add(controls["minimize_tray"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["minimize_on_startup"] = wx.CheckBox( - panel, label="Start minimized to notification area" + panel, + label="Start minimized to the notification area", ) - sizer.Add(controls["minimize_on_startup"], 0, wx.LEFT | wx.BOTTOM, 5) - - controls["startup"] = wx.CheckBox(panel, label="Launch automatically at startup") - sizer.Add(controls["startup"], 0, wx.LEFT | wx.BOTTOM, 5) - - controls["weather_history"] = wx.CheckBox(panel, label="Enable weather history comparisons") - sizer.Add(controls["weather_history"], 0, wx.LEFT | wx.BOTTOM, 5) - - # Reset Configuration - sizer.Add(wx.StaticText(panel, label="Reset Configuration"), 0, wx.LEFT | wx.TOP, 5) - sizer.Add( - wx.StaticText(panel, label="Restore all settings to their default values."), + startup_section.Add( + controls["minimize_on_startup"], 0, - wx.LEFT, - 5, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, ) - - reset_btn = wx.Button(panel, label="Reset all settings to defaults") - reset_btn.Bind(wx.EVT_BUTTON, self.dialog._on_reset_defaults) - sizer.Add(reset_btn, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) - - # Full Data Reset - sizer.Add(wx.StaticText(panel, label="Full Data Reset"), 0, wx.LEFT, 5) - sizer.Add( - wx.StaticText( - panel, - label="Reset all application data: settings, locations, caches, and alert state.", - ), + controls["startup"] = wx.CheckBox(panel, label="Launch automatically at startup") + startup_section.Add(controls["startup"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + controls["weather_history"] = wx.CheckBox( + panel, + label="Enable weather history comparisons", + ) + startup_section.Add( + controls["weather_history"], 0, - wx.LEFT, - 5, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, ) - full_reset_btn = wx.Button(panel, label="Reset all app data (settings, locations, caches)") - full_reset_btn.Bind(wx.EVT_BUTTON, self.dialog._on_full_reset) - sizer.Add(full_reset_btn, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) - - # Configuration Files - sizer.Add(wx.StaticText(panel, label="Configuration Files"), 0, wx.LEFT, 5) - - open_config_btn = wx.Button(panel, label="Open current config directory") - open_config_btn.Bind(wx.EVT_BUTTON, self.dialog._on_open_config_dir) - sizer.Add(open_config_btn, 0, wx.LEFT | wx.TOP, 10) - - open_installed_config_btn = wx.Button( - panel, label="Open installed config directory (source)" + backup_section = self.dialog.create_section( + panel, + sizer, + "Backup and transfer", + "Use these tools when you want to move settings or encrypted API keys between installations.", ) - open_installed_config_btn.Bind(wx.EVT_BUTTON, self.dialog._on_open_installed_config_dir) - sizer.Add(open_installed_config_btn, 0, wx.LEFT | wx.TOP, 10) - - if self.dialog._is_runtime_portable_mode(): - migrate_config_btn = wx.Button(panel, label="Copy installed config to portable") - migrate_config_btn.Bind( - wx.EVT_BUTTON, self.dialog._on_copy_installed_config_to_portable - ) - sizer.Add(migrate_config_btn, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) - - # Settings Backup - sizer.Add(wx.StaticText(panel, label="Settings Backup"), 0, wx.LEFT | wx.TOP, 5) - - export_btn = wx.Button(panel, label="Export Settings...") + export_btn = wx.Button(panel, label="Export settings...") export_btn.Bind(wx.EVT_BUTTON, self.dialog._on_export_settings) - sizer.Add(export_btn, 0, wx.LEFT | wx.TOP, 10) - - import_btn = wx.Button(panel, label="Import Settings...") + backup_section.Add(export_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + import_btn = wx.Button(panel, label="Import settings...") import_btn.Bind(wx.EVT_BUTTON, self.dialog._on_import_settings) - sizer.Add(import_btn, 0, wx.LEFT | wx.TOP, 10) - - sizer.Add( - wx.StaticText(panel, label="API Key Portability (Encrypted)"), - 0, - wx.LEFT | wx.TOP, - 5, - ) - sizer.Add( - wx.StaticText( - panel, - label=( - "Optional: export API keys to an encrypted bundle for transfer. " - "Import requires your passphrase and stores keys in this machine's keyring." - ), - ), - 0, - wx.LEFT, - 5, - ) - + backup_section.Add(import_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) export_api_keys_btn = wx.Button(panel, label="Export API keys (encrypted)") export_api_keys_btn.Bind(wx.EVT_BUTTON, self.dialog._on_export_encrypted_api_keys) - sizer.Add(export_api_keys_btn, 0, wx.LEFT | wx.TOP, 10) - + backup_section.Add(export_api_keys_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) import_api_keys_btn = wx.Button(panel, label="Import API keys (encrypted)") import_api_keys_btn.Bind(wx.EVT_BUTTON, self.dialog._on_import_encrypted_api_keys) - sizer.Add(import_api_keys_btn, 0, wx.LEFT | wx.TOP, 10) - - # Sound Pack Files - sizer.Add(wx.StaticText(panel, label="Sound Pack Files"), 0, wx.LEFT | wx.TOP, 5) + backup_section.Add(import_api_keys_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + folders_section = self.dialog.create_section( + panel, + sizer, + "Folders and files", + "Open the locations where AccessiWeather stores configuration or sound pack files.", + ) + open_config_btn = wx.Button(panel, label="Open current config folder") + open_config_btn.Bind(wx.EVT_BUTTON, self.dialog._on_open_config_dir) + folders_section.Add(open_config_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + open_installed_config_btn = wx.Button( + panel, + label="Open installed config folder (source)", + ) + open_installed_config_btn.Bind( + wx.EVT_BUTTON, + self.dialog._on_open_installed_config_dir, + ) + folders_section.Add( + open_installed_config_btn, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, + ) + if self.dialog._is_runtime_portable_mode(): + migrate_config_btn = wx.Button( + panel, + label="Copy installed config to portable", + ) + migrate_config_btn.Bind( + wx.EVT_BUTTON, + self.dialog._on_copy_installed_config_to_portable, + ) + folders_section.Add( + migrate_config_btn, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, + ) open_sounds_btn = wx.Button(panel, label="Open sound packs folder") open_sounds_btn.Bind(wx.EVT_BUTTON, self.dialog._on_open_soundpacks_dir) - sizer.Add(open_sounds_btn, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) + folders_section.Add(open_sounds_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + + reset_section = self.dialog.create_section( + panel, + sizer, + "Reset and maintenance", + "Use these actions carefully. Resetting all app data removes settings, saved locations, caches, and alert state.", + ) + reset_btn = wx.Button(panel, label="Reset settings to defaults") + reset_btn.Bind(wx.EVT_BUTTON, self.dialog._on_reset_defaults) + reset_section.Add(reset_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + full_reset_btn = wx.Button( + panel, + label="Reset all app data (settings, locations, caches)", + ) + full_reset_btn.Bind(wx.EVT_BUTTON, self.dialog._on_full_reset) + reset_section.Add(full_reset_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Advanced") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -165,8 +175,8 @@ def setup_accessibility(self): """Set accessibility names for Advanced tab controls.""" controls = self.dialog._controls names = { - "minimize_tray": "Minimize to notification area when closing", - "minimize_on_startup": "Start minimized to notification area", + "minimize_tray": "Minimize to the notification area when closing", + "minimize_on_startup": "Start minimized to the notification area", "startup": "Launch automatically at startup", "weather_history": "Enable weather history comparisons", } diff --git a/src/accessiweather/ui/dialogs/settings_tabs/ai.py b/src/accessiweather/ui/dialogs/settings_tabs/ai.py index a7076ed2..80553a9a 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/ai.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/ai.py @@ -19,61 +19,61 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "AI"): """Build the AI tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - sizer.Add(wx.StaticText(panel, label="AI Weather Explanations"), 0, wx.ALL, 5) - sizer.Add( - wx.StaticText( - panel, - label="Get natural language explanations of weather conditions using AI.", - ), - 0, - wx.LEFT | wx.BOTTOM, - 5, + self.dialog.add_help_text( + panel, + sizer, + "Use AI explanations if you want plain-language weather summaries powered by OpenRouter.", + left=5, ) - sizer.Add(wx.StaticText(panel, label="OpenRouter API Key:"), 0, wx.ALL, 5) - controls["openrouter_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(300, -1)) - sizer.Add(controls["openrouter_key"], 0, wx.LEFT, 10) - - validate_btn = wx.Button(panel, label="Validate API Key") + key_section = self.dialog.create_section( + panel, + sizer, + "OpenRouter access", + "Add and validate your OpenRouter API key before choosing a model.", + ) + controls["openrouter_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(320, -1)) + self.dialog.add_labeled_row( + panel, + key_section, + "OpenRouter API key:", + controls["openrouter_key"], + expand_control=True, + ) + validate_btn = wx.Button(panel, label="Validate OpenRouter key") validate_btn.Bind(wx.EVT_BUTTON, self.dialog._on_validate_openrouter_key) - sizer.Add(validate_btn, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) + key_section.Add(validate_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) - row_model = wx.BoxSizer(wx.HORIZONTAL) - row_model.Add( - wx.StaticText(panel, label="Model Preference:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + model_section = self.dialog.create_section( + panel, + sizer, + "Model and explanation style", + "Free options are easiest to start with. Paid routing can unlock more model choices.", ) controls["ai_model"] = wx.Choice( panel, choices=[ - "Free Router (Auto, Free)", - "Llama 3.3 70B (Free)", - "Auto Router (Paid)", + "Free router (automatic, free)", + "Llama 3.3 70B (free)", + "Auto router (paid)", ], ) - row_model.Add(controls["ai_model"], 0) - sizer.Add(row_model, 0, wx.LEFT, 10) - - browse_btn = wx.Button(panel, label="Browse Models...") - browse_btn.Bind(wx.EVT_BUTTON, self.dialog._on_browse_models) - sizer.Add(browse_btn, 0, wx.LEFT | wx.TOP, 10) - - row_style = wx.BoxSizer(wx.HORIZONTAL) - row_style.Add( - wx.StaticText(panel, label="Explanation Style:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + self.dialog.add_labeled_row( + panel, + model_section, + "Model preference:", + controls["ai_model"], ) + browse_btn = wx.Button(panel, label="Browse OpenRouter models...") + browse_btn.Bind(wx.EVT_BUTTON, self.dialog._on_browse_models) + model_section.Add(browse_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["ai_style"] = wx.Choice( panel, choices=[ @@ -82,50 +82,70 @@ def create(self): "Detailed (full paragraph)", ], ) - row_style.Add(controls["ai_style"], 0) - sizer.Add(row_style, 0, wx.LEFT | wx.TOP, 10) + self.dialog.add_labeled_row( + panel, + model_section, + "Explanation style:", + controls["ai_style"], + ) - sizer.Add( - wx.StaticText(panel, label="Custom System Prompt (optional):"), + prompt_section = self.dialog.create_section( + panel, + sizer, + "Custom prompts", + "Leave these blank to use the default behavior. Add text only if you want more control over tone or focus.", + ) + controls["custom_prompt"] = wx.TextCtrl(panel, style=wx.TE_MULTILINE, size=(420, 70)) + prompt_section.Add( + wx.StaticText(panel, label="Custom system prompt (optional):"), 0, - wx.LEFT | wx.TOP, + wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) - controls["custom_prompt"] = wx.TextCtrl(panel, style=wx.TE_MULTILINE, size=(400, 60)) - sizer.Add(controls["custom_prompt"], 0, wx.LEFT | wx.EXPAND, 10) - - reset_prompt_btn = wx.Button(panel, label="Reset to Default") - reset_prompt_btn.Bind(wx.EVT_BUTTON, self.dialog._on_reset_prompt) - sizer.Add(reset_prompt_btn, 0, wx.LEFT | wx.TOP, 10) - - sizer.Add( - wx.StaticText(panel, label="Custom Instructions (optional):"), + prompt_section.Add( + controls["custom_prompt"], 0, - wx.LEFT | wx.TOP, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - controls["custom_instructions"] = wx.TextCtrl(panel, style=wx.TE_MULTILINE, size=(400, 40)) + reset_prompt_btn = wx.Button(panel, label="Reset prompt to default") + reset_prompt_btn.Bind(wx.EVT_BUTTON, self.dialog._on_reset_prompt) + prompt_section.Add(reset_prompt_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + controls["custom_instructions"] = wx.TextCtrl( + panel, + style=wx.TE_MULTILINE, + size=(420, 50), + ) controls["custom_instructions"].SetHint( - "e.g., Focus on outdoor activities, Keep responses under 50 words" + "For example: focus on outdoor activities, keep responses under 50 words" ) - sizer.Add(controls["custom_instructions"], 0, wx.LEFT | wx.EXPAND, 10) - - sizer.Add(wx.StaticText(panel, label="Cost Information:"), 0, wx.LEFT | wx.TOP, 10) - sizer.Add( - wx.StaticText(panel, label="Free models: No cost, may have rate limits"), + prompt_section.Add( + wx.StaticText(panel, label="Custom instructions (optional):"), 0, - wx.LEFT, - 15, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, ) - sizer.Add( - wx.StaticText(panel, label="Paid models: ~$0.001 per explanation (varies by model)"), + prompt_section.Add( + controls["custom_instructions"], 0, - wx.LEFT, - 15, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) + + cost_section = self.dialog.create_section( + panel, + sizer, + "Cost notes", + None, + ) + self.dialog.add_help_text( + panel, + cost_section, + "Free models do not cost money but may be rate limited. Paid models often cost around $0.001 per explanation, depending on the model.", ) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "AI") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -139,7 +159,7 @@ def load(self, settings): if hasattr(wx, "EVT_TEXT"): controls["openrouter_key"].Bind( wx.EVT_TEXT, - lambda e: setattr(self.dialog, "_openrouter_key_cleared", True), + lambda _event: setattr(self.dialog, "_openrouter_key_cleared", True), ) ai_model = getattr(settings, "ai_model_preference", "openrouter/free") @@ -179,11 +199,11 @@ def setup_accessibility(self): """Set accessibility names for AI tab controls.""" controls = self.dialog._controls names = { - "openrouter_key": "OpenRouter API Key", - "ai_model": "Model Preference", - "ai_style": "Explanation Style", - "custom_prompt": "Custom System Prompt (optional)", - "custom_instructions": "Custom Instructions (optional)", + "openrouter_key": "OpenRouter API key", + "ai_model": "AI model preference", + "ai_style": "AI explanation style", + "custom_prompt": "Custom system prompt", + "custom_instructions": "Custom instructions", } for key, name in names.items(): controls[key].SetName(name) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/audio.py b/src/accessiweather/ui/dialogs/settings_tabs/audio.py index 857b4d65..63ea06ae 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/audio.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/audio.py @@ -16,10 +16,6 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - # ------------------------------------------------------------------ - # Static helpers (also used by SettingsDialogSimple for backward compat) - # ------------------------------------------------------------------ - @staticmethod def _get_event_sound_sections() -> tuple[tuple[str, str, tuple[str, ...]], ...]: """Return grouped event-sound sections used by the settings UI.""" @@ -48,9 +44,16 @@ def _build_default_event_sound_states(cls) -> dict[str, bool]: """Return the default enabled state for each mutable sound event.""" return {event_key: True for event_key, _label in cls._get_mutable_sound_events()} - # ------------------------------------------------------------------ - # Instance helpers (read/write dialog state) - # ------------------------------------------------------------------ + @classmethod + def build_event_sound_summary_text(cls, state_map: dict[str, bool]) -> str: + """Build plain-language event sound summary text.""" + total_events = len(cls._get_mutable_sound_events()) + enabled_count = sum(1 for enabled in state_map.values() if enabled) + if enabled_count == total_events: + return f"Sounds will play for all {total_events} selectable event types." + if enabled_count == 0: + return "Sounds are turned off for every selectable event type." + return f"Sounds will play for {enabled_count} of {total_events} selectable event types." def _get_muted_sound_events(self) -> list[str]: """Return muted event keys in the configured display order.""" @@ -66,15 +69,11 @@ def _get_muted_sound_events(self) -> list[str]: def _get_event_sound_summary_text(self) -> str: """Build summary text shown on the audio tab.""" - total_events = len(self._get_mutable_sound_events()) - muted_events = self._get_muted_sound_events() - enabled_count = total_events - len(muted_events) - - if not muted_events: - return f"All {total_events} sound events are enabled." - if enabled_count == 0: - return "All event sounds are turned off." - return f"{enabled_count} of {total_events} sound events are enabled." + state_map = ( + getattr(self.dialog, "_event_sound_states", {}) + or self._build_default_event_sound_states() + ) + return self.build_event_sound_summary_text(state_map) def _refresh_event_sound_summary(self) -> None: """Refresh the event sound summary shown on the audio tab.""" @@ -91,44 +90,34 @@ def set_event_sound_states(self, muted_sound_events: list[str] | tuple[str, ...] } self._refresh_event_sound_summary() - # ------------------------------------------------------------------ - # Tab interface - # ------------------------------------------------------------------ - - def create(self): + def create(self, page_label: str = "Audio"): """Build the Audio tab panel and add it to the notebook.""" - panel = wx.Panel(self.dialog.notebook) + panel = wx.ScrolledWindow(self.dialog.notebook) + panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - sizer.Add(wx.StaticText(panel, label="Audio"), 0, wx.ALL, 5) - sizer.Add( - wx.StaticText( - panel, - label=( - "Choose whether sounds are enabled, which sound pack is active, " - "and which events should play audio." - ), - ), - 0, - wx.LEFT | wx.RIGHT | wx.BOTTOM, - 5, + self.dialog.add_help_text( + panel, + sizer, + "Choose whether AccessiWeather plays sounds, which sound pack it uses, and which events are allowed to make noise.", + left=5, ) - playback_section = wx.StaticBoxSizer(wx.VERTICAL, panel, "Playback") - + playback_section = self.dialog.create_section( + panel, + sizer, + "Playback", + "Turn sound on or off, then choose the sound pack you want to hear.", + ) controls["sound_enabled"] = wx.CheckBox(panel, label="Play notification sounds") - playback_section.Add(controls["sound_enabled"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) - - row1 = wx.BoxSizer(wx.HORIZONTAL) - row1.Add( - wx.StaticText(panel, label="Sound pack:"), + playback_section.Add( + controls["sound_enabled"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - # Load available sound packs self.dialog._sound_pack_ids = ["default"] pack_names = ["Default"] try: @@ -141,42 +130,51 @@ def create(self): logger.warning(f"Failed to load sound packs: {e}") controls["sound_pack"] = wx.Choice(panel, choices=pack_names) - row1.Add(controls["sound_pack"], 0) - playback_section.Add(row1, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) - - test_btn = wx.Button(panel, label="Test Sound") - test_btn.Bind(wx.EVT_BUTTON, self.dialog._on_test_sound) + self.dialog.add_labeled_row( + panel, + playback_section, + "Sound pack:", + controls["sound_pack"], + ) action_row = wx.BoxSizer(wx.HORIZONTAL) + test_btn = wx.Button(panel, label="Play sample sound") + test_btn.Bind(wx.EVT_BUTTON, self.dialog._on_test_sound) action_row.Add(test_btn, 0, wx.RIGHT, 10) - - manage_btn = wx.Button(panel, label="Manage Sound Packs...") + manage_btn = wx.Button(panel, label="Manage sound packs...") manage_btn.Bind(wx.EVT_BUTTON, self.dialog._on_manage_soundpacks) action_row.Add(manage_btn, 0) - playback_section.Add(action_row, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) - sizer.Add(playback_section, 0, wx.EXPAND | wx.ALL, 5) + playback_section.Add(action_row, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) - event_sounds_section = wx.StaticBoxSizer(wx.VERTICAL, panel, "Event sounds") - event_sounds_section.Add( - wx.StaticText(panel, label="Choose which events can play sounds."), - 0, - wx.LEFT | wx.RIGHT | wx.BOTTOM, - 5, + event_sounds_section = self.dialog.create_section( + panel, + sizer, + "When sounds play", + "Choose which event types can make sound. This gives you finer control without turning all audio off.", ) controls["event_sounds_summary"] = wx.StaticText(panel, label="") event_sounds_section.Add( - controls["event_sounds_summary"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5 + controls["event_sounds_summary"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) + controls["configure_event_sounds"] = wx.Button( + panel, + label="Choose event sounds...", ) - controls["configure_event_sounds"] = wx.Button(panel, label="Configure Event Sounds...") controls["configure_event_sounds"].Bind( - wx.EVT_BUTTON, self.dialog._on_configure_event_sounds + wx.EVT_BUTTON, + self.dialog._on_configure_event_sounds, ) event_sounds_section.Add( - controls["configure_event_sounds"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5 + controls["configure_event_sounds"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, ) - sizer.Add(event_sounds_section, 0, wx.EXPAND | wx.ALL, 5) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Audio") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -212,10 +210,10 @@ def setup_accessibility(self): """Set accessibility names for Audio tab controls.""" controls = self.dialog._controls names = { - "sound_enabled": "Enable Sounds", - "sound_pack": "Active sound pack", + "sound_enabled": "Play notification sounds", + "sound_pack": "Sound pack", "event_sounds_summary": "Event sound summary", - "configure_event_sounds": "Configure event sounds", + "configure_event_sounds": "Choose event sounds", } for key, name in names.items(): controls[key].SetName(name) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py b/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py index b02ae6e7..08f497c0 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py @@ -16,6 +16,12 @@ "major_airport_preferred", "freshest_observation", ] +_STATION_STRATEGY_LABELS = [ + "Hybrid default", + "Nearest station", + "Major airport preferred", + "Freshest observation", +] class DataSourcesTab: @@ -36,114 +42,184 @@ def _build_default_source_settings_states() -> dict: "station_selection_strategy": 0, } - def create(self): + @staticmethod + def build_source_settings_summary_text(state: dict) -> str: + """Build plain-language summary text shown on the data sources tab.""" + strat_idx = state.get("station_selection_strategy", 0) + if 0 <= strat_idx < len(_STATION_STRATEGY_LABELS): + strat_text = _STATION_STRATEGY_LABELS[strat_idx] + else: + strat_text = _STATION_STRATEGY_LABELS[0] + + sources = ["NWS", "Open-Meteo", "Visual Crossing", "Pirate Weather"] + keys = [ + "auto_use_nws", + "auto_use_openmeteo", + "auto_use_visualcrossing", + "auto_use_pirateweather", + ] + enabled = [s for s, k in zip(sources, keys, strict=True) if state.get(k, True)] + enabled_text = ", ".join(enabled) if enabled else "Open-Meteo only" + return f"Automatic mode uses: {enabled_text}. NWS station strategy: {strat_text}." + + def create(self, page_label: str = "Data Sources"): """Build the Data Sources tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - # Data source selection - row1 = wx.BoxSizer(wx.HORIZONTAL) - row1.Add( - wx.StaticText(panel, label="Weather Data Source:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + self.dialog.add_help_text( + panel, + sizer, + "Choose where weather data comes from and manage provider-specific API keys.", + left=5, + ) + + source_section = self.dialog.create_section( + panel, + sizer, + "Choose a weather source", + "Automatic mode combines available providers. Single-source options keep behavior predictable when you prefer one provider.", ) controls["data_source"] = wx.Choice( panel, choices=[ - "Automatic (merges all available sources)", - "National Weather Service (US only, forecast + alerts)", - "Open-Meteo (Global forecast, no alerts, no API key)", - "Visual Crossing (Global forecast, US/Canada/Europe alerts, API key)", - "Pirate Weather (Global forecast + worldwide alerts, API key)", + "Automatic (combine all available sources)", + "National Weather Service (US only, forecast and alerts)", + "Open-Meteo (global forecast, no alerts, no API key)", + "Visual Crossing (global forecast, regional alerts, API key)", + "Pirate Weather (global forecast and alerts, API key)", ], ) - row1.Add(controls["data_source"], 0) controls["data_source"].Bind(wx.EVT_CHOICE, self.dialog._on_data_source_changed) - sizer.Add(row1, 0, wx.ALL, 5) + self.dialog.add_labeled_row( + panel, + source_section, + "Weather source:", + controls["data_source"], + ) - # Visual Crossing Configuration - self.dialog._vc_config_sizer = wx.BoxSizer(wx.VERTICAL) - self.dialog._vc_config_sizer.Add( - wx.StaticText(panel, label="Visual Crossing API Configuration:"), + auto_section = self.dialog.create_section( + panel, + sizer, + "Automatic mode", + "Fine-tune which sources Automatic mode can use and how NWS picks a station for current conditions.", + ) + controls["source_settings_summary"] = wx.TextCtrl( + panel, + value=self._get_source_settings_summary_text(), + size=(-1, 52), + style=wx.TE_MULTILINE | wx.TE_NO_VSCROLL | wx.TE_READONLY, + ) + auto_section.Add( + controls["source_settings_summary"], 0, - wx.ALL, - 5, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - - row_key = wx.BoxSizer(wx.HORIZONTAL) - row_key.Add( - wx.StaticText(panel, label="Visual Crossing API Key:"), + controls["configure_source_settings"] = wx.Button( + panel, + label="Choose automatic mode sources...", + ) + controls["configure_source_settings"].Bind( + wx.EVT_BUTTON, + self.dialog._on_configure_source_settings, + ) + auto_section.Add( + controls["configure_source_settings"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) - controls["vc_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(250, -1)) - row_key.Add(controls["vc_key"], 1) - self.dialog._vc_config_sizer.Add(row_key, 0, wx.LEFT | wx.EXPAND, 10) - btn_row = wx.BoxSizer(wx.HORIZONTAL) - get_key_btn = wx.Button(panel, label="Get Free API Key") - get_key_btn.Bind(wx.EVT_BUTTON, self.dialog._on_get_vc_api_key) - btn_row.Add(get_key_btn, 0, wx.RIGHT, 10) - validate_btn = wx.Button(panel, label="Validate API Key") - validate_btn.Bind(wx.EVT_BUTTON, self.dialog._on_validate_vc_api_key) - btn_row.Add(validate_btn, 0) - self.dialog._vc_config_sizer.Add(btn_row, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) - sizer.Add(self.dialog._vc_config_sizer, 0, wx.EXPAND) + keys_section = self.dialog.create_section( + panel, + sizer, + "Provider API keys", + "Only Visual Crossing and Pirate Weather need keys. Stored keys stay in secure storage unless you explicitly export them.", + ) - # Pirate Weather Configuration - self.dialog._pw_config_sizer = wx.BoxSizer(wx.VERTICAL) - self.dialog._pw_config_sizer.Add( - wx.StaticText(panel, label="Pirate Weather API Configuration:"), + self.dialog._vc_config_sizer = wx.StaticBoxSizer( + wx.VERTICAL, + panel, + "Visual Crossing", + ) + vc_key_help = wx.StaticText( + panel, + label="Use this provider for global forecasts and regional alerts where available.", + ) + self.dialog._wrap_static_text(vc_key_help) + self.dialog._vc_config_sizer.Add( + vc_key_help, 0, - wx.ALL, - 5, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - - row_pw_key = wx.BoxSizer(wx.HORIZONTAL) - row_pw_key.Add( - wx.StaticText(panel, label="Pirate Weather API Key:"), + controls["vc_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(280, -1)) + self.dialog.add_labeled_row( + panel, + self.dialog._vc_config_sizer, + "Visual Crossing API key:", + controls["vc_key"], + expand_control=True, + ) + vc_button_row = wx.BoxSizer(wx.HORIZONTAL) + get_key_btn = wx.Button(panel, label="Get Visual Crossing key") + get_key_btn.Bind(wx.EVT_BUTTON, self.dialog._on_get_vc_api_key) + vc_button_row.Add(get_key_btn, 0, wx.RIGHT, 10) + validate_btn = wx.Button(panel, label="Validate Visual Crossing key") + validate_btn.Bind(wx.EVT_BUTTON, self.dialog._on_validate_vc_api_key) + vc_button_row.Add(validate_btn, 0) + self.dialog._vc_config_sizer.Add( + vc_button_row, 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) - controls["pw_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(250, -1)) - row_pw_key.Add(controls["pw_key"], 1) - self.dialog._pw_config_sizer.Add(row_pw_key, 0, wx.LEFT | wx.EXPAND, 10) + keys_section.Add(self.dialog._vc_config_sizer, 0, wx.EXPAND | wx.BOTTOM, 8) - btn_row_pw = wx.BoxSizer(wx.HORIZONTAL) - get_pw_key_btn = wx.Button(panel, label="Get Free API Key") - get_pw_key_btn.Bind(wx.EVT_BUTTON, self.dialog._on_get_pw_api_key) - btn_row_pw.Add(get_pw_key_btn, 0, wx.RIGHT, 10) - validate_pw_btn = wx.Button(panel, label="Validate API Key") - validate_pw_btn.Bind(wx.EVT_BUTTON, self.dialog._on_validate_pw_api_key) - btn_row_pw.Add(validate_pw_btn, 0) - self.dialog._pw_config_sizer.Add(btn_row_pw, 0, wx.LEFT | wx.TOP | wx.BOTTOM, 10) - sizer.Add(self.dialog._pw_config_sizer, 0, wx.EXPAND) - - # Source Settings summary + button - sizer.Add(wx.StaticText(panel, label="Source Settings:"), 0, wx.ALL, 5) - controls["source_settings_summary"] = wx.TextCtrl( + self.dialog._pw_config_sizer = wx.StaticBoxSizer( + wx.VERTICAL, panel, - value=self._get_source_settings_summary_text(), - size=(-1, 44), - style=wx.TE_READONLY | wx.TE_MULTILINE | wx.TE_NO_VSCROLL, + "Pirate Weather", ) - sizer.Add(controls["source_settings_summary"], 0, wx.LEFT | wx.BOTTOM | wx.EXPAND, 5) - controls["configure_source_settings"] = wx.Button( - panel, label="Configure Source Settings..." + pw_key_help = wx.StaticText( + panel, + label="Use this provider for global forecasts and broader alert coverage.", ) - controls["configure_source_settings"].Bind( - wx.EVT_BUTTON, self.dialog._on_configure_source_settings + self.dialog._wrap_static_text(pw_key_help) + self.dialog._pw_config_sizer.Add( + pw_key_help, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) + controls["pw_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(280, -1)) + self.dialog.add_labeled_row( + panel, + self.dialog._pw_config_sizer, + "Pirate Weather API key:", + controls["pw_key"], + expand_control=True, + ) + pw_button_row = wx.BoxSizer(wx.HORIZONTAL) + get_pw_key_btn = wx.Button(panel, label="Get Pirate Weather key") + get_pw_key_btn.Bind(wx.EVT_BUTTON, self.dialog._on_get_pw_api_key) + pw_button_row.Add(get_pw_key_btn, 0, wx.RIGHT, 10) + validate_pw_btn = wx.Button(panel, label="Validate Pirate Weather key") + validate_pw_btn.Bind(wx.EVT_BUTTON, self.dialog._on_validate_pw_api_key) + pw_button_row.Add(validate_pw_btn, 0) + self.dialog._pw_config_sizer.Add( + pw_button_row, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, ) - sizer.Add(controls["configure_source_settings"], 0, wx.LEFT | wx.BOTTOM, 5) + keys_section.Add(self.dialog._pw_config_sizer, 0, wx.EXPAND) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Data Sources") + self.dialog.notebook.AddPage(panel, page_label) return panel def _get_source_settings_summary_text(self) -> str: @@ -152,28 +228,7 @@ def _get_source_settings_summary_text(self) -> str: getattr(self.dialog, "_source_settings_states", None) or self._build_default_source_settings_states() ) - strategy_labels = [ - "Hybrid default", - "Nearest station", - "Major airport preferred", - "Freshest observation", - ] - strat_idx = state.get("station_selection_strategy", 0) - strat_text = ( - strategy_labels[strat_idx] - if 0 <= strat_idx < len(strategy_labels) - else strategy_labels[0] - ) - sources = ["NWS", "Open-Meteo", "Visual Crossing", "Pirate Weather"] - keys = [ - "auto_use_nws", - "auto_use_openmeteo", - "auto_use_visualcrossing", - "auto_use_pirateweather", - ] - enabled = [s for s, k in zip(sources, keys, strict=True) if state.get(k, True)] - enabled_text = ", ".join(enabled) if enabled else "None" - return f"Auto sources: {enabled_text} | Station: {strat_text}" + return self.build_source_settings_summary_text(state) def refresh_source_settings_summary(self) -> None: """Refresh the source settings summary control.""" @@ -194,7 +249,7 @@ def load(self, settings): self.dialog._vc_key_cleared = False if hasattr(wx, "EVT_TEXT"): - def _on_vc_key_text(e, _dlg=self.dialog): + def _on_vc_key_text(_event, _dlg=self.dialog): _dlg._vc_key_cleared = True _dlg._update_auto_source_key_state() @@ -206,13 +261,12 @@ def _on_vc_key_text(e, _dlg=self.dialog): self.dialog._pw_key_cleared = False if hasattr(wx, "EVT_TEXT"): - def _on_pw_key_text(e, _dlg=self.dialog): + def _on_pw_key_text(_event, _dlg=self.dialog): _dlg._pw_key_cleared = True _dlg._update_auto_source_key_state() controls["pw_key"].Bind(wx.EVT_TEXT, _on_pw_key_text) - # Source settings sub-dialog state saved_strategy = getattr(settings, "station_selection_strategy", "hybrid_default") strat_idx = ( _STATION_STRATEGY_VALUES.index(saved_strategy) @@ -249,7 +303,7 @@ def save(self) -> dict: controls = self.dialog._controls state = getattr(self.dialog, "_source_settings_states", None) or {} - def _src_enabled(source_key: str, flag_key: str) -> bool: + def _src_enabled(_source_key: str, flag_key: str) -> bool: return state.get(flag_key, True) source_priority_us = [ @@ -291,11 +345,11 @@ def setup_accessibility(self): """Set accessibility names for Data Sources tab controls.""" controls = self.dialog._controls names = { - "data_source": "Weather Data Source", - "vc_key": "Visual Crossing API Key", - "pw_key": "Pirate Weather API Key", - "source_settings_summary": "Source settings summary", - "configure_source_settings": "Configure source settings", + "data_source": "Weather source", + "vc_key": "Visual Crossing API key", + "pw_key": "Pirate Weather API key", + "source_settings_summary": "Automatic mode source summary", + "configure_source_settings": "Choose automatic mode sources", } for key, name in names.items(): controls[key].SetName(name) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index be55c961..553e4547 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -27,15 +27,26 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "Display"): """Build the Display tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - # Unit Preference - sizer.Add(wx.StaticText(panel, label="Unit Preference:"), 0, wx.ALL | wx.EXPAND, 5) + self.dialog.add_help_text( + panel, + sizer, + "Choose what weather details appear and how forecast times and units are shown.", + left=5, + ) + + units_section = self.dialog.create_section( + panel, + sizer, + "Units and forecast length", + "These settings control the overall scale and amount of forecast information you see.", + ) controls["temp_unit"] = wx.Choice( panel, choices=[ @@ -45,115 +56,123 @@ def create(self): "Both (°F and °C)", ], ) - sizer.Add(controls["temp_unit"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) - - # Metric Visibility - sizer.Add(wx.StaticText(panel, label="Metric Visibility:", style=wx.BOLD), 0, wx.ALL, 5) - sizer.Add( - wx.StaticText(panel, label="Select which weather metrics to display:"), + self.dialog.add_labeled_row( + panel, + units_section, + "Temperature units:", + controls["temp_unit"], + ) + controls["forecast_duration_days"] = wx.Choice( + panel, + choices=["3 days", "5 days", "7 days (default)", "10 days", "14 days", "15 days"], + ) + self.dialog.add_labeled_row( + panel, + units_section, + "Forecast length:", + controls["forecast_duration_days"], + ) + controls["hourly_forecast_hours"] = wx.SpinCtrl(panel, min=1, max=168, initial=6) + self.dialog.add_labeled_row( + panel, + units_section, + "Hourly forecast hours:", + controls["hourly_forecast_hours"], + ) + controls["round_values"] = wx.CheckBox( + panel, + label="Show values as whole numbers when possible", + ) + units_section.Add( + controls["round_values"], 0, - wx.LEFT | wx.BOTTOM, - 5, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - controls["show_dewpoint"] = wx.CheckBox(panel, label="Show dewpoint") - sizer.Add(controls["show_dewpoint"], 0, wx.LEFT, 10) - + details_section = self.dialog.create_section( + panel, + sizer, + "Extra weather details", + "Turn on the details that matter to you most.", + ) + controls["show_dewpoint"] = wx.CheckBox(panel, label="Show dew point") + details_section.Add(controls["show_dewpoint"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["show_visibility"] = wx.CheckBox(panel, label="Show visibility") - sizer.Add(controls["show_visibility"], 0, wx.LEFT, 10) - + details_section.Add(controls["show_visibility"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["show_uv_index"] = wx.CheckBox(panel, label="Show UV index") - sizer.Add(controls["show_uv_index"], 0, wx.LEFT, 10) - + details_section.Add(controls["show_uv_index"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["show_pressure_trend"] = wx.CheckBox(panel, label="Show pressure trend") - sizer.Add(controls["show_pressure_trend"], 0, wx.LEFT, 10) - - controls["show_impact_summaries"] = wx.CheckBox( - panel, label="Show weather impact analysis (Outdoor, Driving, Allergy)" - ) - sizer.Add(controls["show_impact_summaries"], 0, wx.LEFT, 10) - - controls["round_values"] = wx.CheckBox( - panel, label="Show values as whole numbers (no decimals)" - ) - sizer.Add(controls["round_values"], 0, wx.LEFT | wx.TOP, 10) - - row_forecast_duration = wx.BoxSizer(wx.HORIZONTAL) - row_forecast_duration.Add( - wx.StaticText(panel, label="Forecast duration:"), + details_section.Add( + controls["show_pressure_trend"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) - controls["forecast_duration_days"] = wx.Choice( + controls["show_impact_summaries"] = wx.CheckBox( panel, - choices=["3 days", "5 days", "7 days (default)", "10 days", "14 days", "15 days"], + label="Show impact summaries for outdoor, driving, and allergy conditions", ) - row_forecast_duration.Add(controls["forecast_duration_days"], 0) - sizer.Add(row_forecast_duration, 0, wx.LEFT | wx.TOP, 10) - - row_hourly_hours = wx.BoxSizer(wx.HORIZONTAL) - row_hourly_hours.Add( - wx.StaticText(panel, label="Hourly forecast hours:"), + details_section.Add( + controls["show_impact_summaries"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - controls["hourly_forecast_hours"] = wx.SpinCtrl(panel, min=1, max=168, initial=6) - row_hourly_hours.Add(controls["hourly_forecast_hours"], 0) - sizer.Add(row_hourly_hours, 0, wx.LEFT | wx.TOP, 10) - - # Time & Date Display - sizer.Add(wx.StaticText(panel, label="Time & Date Display:"), 0, wx.ALL, 5) - row_time_ref = wx.BoxSizer(wx.HORIZONTAL) - row_time_ref.Add( - wx.StaticText(panel, label="Forecast time display:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + time_section = self.dialog.create_section( + panel, + sizer, + "Time display", + "Choose whose timezone to use and how timestamps should be formatted.", ) controls["forecast_time_reference"] = wx.Choice( panel, - choices=["Location's timezone (default)", "My local timezone"], + choices=["Location timezone (default)", "My local timezone"], ) - row_time_ref.Add(controls["forecast_time_reference"], 0) - sizer.Add(row_time_ref, 0, wx.LEFT, 10) - - row_tz = wx.BoxSizer(wx.HORIZONTAL) - row_tz.Add( - wx.StaticText(panel, label="Time zone display:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + self.dialog.add_labeled_row( + panel, + time_section, + "Forecast times are based on:", + controls["forecast_time_reference"], ) controls["time_display_mode"] = wx.Choice( panel, - choices=["Local time only", "UTC time only", "Both (Local and UTC)"], + choices=["Local time only", "UTC time only", "Both local and UTC"], + ) + self.dialog.add_labeled_row( + panel, + time_section, + "Show times as:", + controls["time_display_mode"], ) - row_tz.Add(controls["time_display_mode"], 0) - sizer.Add(row_tz, 0, wx.LEFT, 10) - controls["time_format_12hour"] = wx.CheckBox( - panel, label="Use 12-hour time format (e.g., 3:00 PM)" + panel, + label="Use 12-hour time format (for example, 3:00 PM)", + ) + time_section.Add( + controls["time_format_12hour"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["time_format_12hour"], 0, wx.LEFT | wx.TOP, 10) - controls["show_timezone_suffix"] = wx.CheckBox( - panel, label="Show timezone abbreviations (e.g., EST, UTC)" + panel, + label="Show timezone abbreviations (for example, EST or UTC)", ) - sizer.Add(controls["show_timezone_suffix"], 0, wx.LEFT, 10) - - # Verbosity - sizer.Add(wx.StaticText(panel, label="Information Priority:"), 0, wx.ALL, 5) - - row_verb = wx.BoxSizer(wx.HORIZONTAL) - row_verb.Add( - wx.StaticText(panel, label="Verbosity level:"), + time_section.Add( + controls["show_timezone_suffix"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) + + priority_section = self.dialog.create_section( + panel, + sizer, + "Reading priority", + "Decide how compact or detailed the spoken and displayed forecast should be.", + ) controls["verbosity_level"] = wx.Choice( panel, choices=[ @@ -162,16 +181,25 @@ def create(self): "Detailed (all available info)", ], ) - row_verb.Add(controls["verbosity_level"], 0) - sizer.Add(row_verb, 0, wx.LEFT, 10) - + self.dialog.add_labeled_row( + panel, + priority_section, + "Verbosity level:", + controls["verbosity_level"], + ) controls["severe_weather_override"] = wx.CheckBox( - panel, label="Automatically prioritize severe weather info" + panel, + label="Automatically prioritize severe weather details", + ) + priority_section.Add( + controls["severe_weather_override"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["severe_weather_override"], 0, wx.ALL, 10) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Display") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -250,20 +278,21 @@ def setup_accessibility(self): """Set accessibility names for Display tab controls.""" controls = self.dialog._controls names = { - "temp_unit": "Unit Preference", - "show_dewpoint": "Show dewpoint", + "temp_unit": "Temperature units", + "show_dewpoint": "Show dew point", "show_visibility": "Show visibility", "show_uv_index": "Show UV index", "show_pressure_trend": "Show pressure trend", - "show_impact_summaries": "Show weather impact analysis (Outdoor, Driving, Allergy)", - "forecast_duration_days": "Forecast duration", + "show_impact_summaries": "Show impact summaries for outdoor driving and allergy conditions", + "round_values": "Show values as whole numbers when possible", + "forecast_duration_days": "Forecast length", "hourly_forecast_hours": "Hourly forecast hours", - "forecast_time_reference": "Forecast time display", - "time_display_mode": "Time zone display", - "time_format_12hour": "Use 12-hour time format (e.g., 3:00 PM)", - "show_timezone_suffix": "Show timezone abbreviations (e.g., EST, UTC)", + "forecast_time_reference": "Forecast time reference", + "time_display_mode": "Time display mode", + "time_format_12hour": "Use 12-hour time format", + "show_timezone_suffix": "Show timezone abbreviations", "verbosity_level": "Verbosity level", - "severe_weather_override": "Automatically prioritize severe weather info", + "severe_weather_override": "Automatically prioritize severe weather details", } for key, name in names.items(): controls[key].SetName(name) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/general.py b/src/accessiweather/ui/dialogs/settings_tabs/general.py index e14e42fa..93b8f011 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/general.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/general.py @@ -16,71 +16,111 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "General"): """Build the General tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - # Update interval - row1 = wx.BoxSizer(wx.HORIZONTAL) - row1.Add( - wx.StaticText(panel, label="Update Interval (minutes):"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + self.dialog.add_help_text( + panel, + sizer, + "Choose how often AccessiWeather refreshes and what appears on the tray icon.", + left=5, + ) + + refresh_section = self.dialog.create_section( + panel, + sizer, + "Weather refresh", + "These settings affect general app behavior for everyday use.", ) controls["update_interval"] = wx.SpinCtrl(panel, min=1, max=120, initial=10) - row1.Add(controls["update_interval"], 0) - sizer.Add(row1, 0, wx.ALL, 5) + self.dialog.add_labeled_row( + panel, + refresh_section, + "Refresh weather every (minutes):", + controls["update_interval"], + ) - # Show Nationwide location controls["show_nationwide"] = wx.CheckBox( - panel, label="Show Nationwide location (requires Auto or NWS data source)" + panel, + label="Show the Nationwide location when a supported data source is selected", + ) + refresh_section.Add( + controls["show_nationwide"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) + self.dialog.add_help_text( + panel, + refresh_section, + "Nationwide is available when your weather source is set to Automatic or NWS.", ) - sizer.Add(controls["show_nationwide"], 0, wx.ALL, 5) - - sizer.Add(wx.StaticLine(panel), 0, wx.EXPAND | wx.ALL, 5) - sizer.Add(wx.StaticText(panel, label="Taskbar icon text:"), 0, wx.LEFT | wx.TOP, 5) + tray_section = self.dialog.create_section( + panel, + sizer, + "Tray icon text", + "Show a short weather summary on the notification-area icon and choose how it updates.", + ) controls["taskbar_icon_text_enabled"] = wx.CheckBox( - panel, label="Show weather text on tray icon" + panel, + label="Show weather text on the tray icon", ) controls["taskbar_icon_text_enabled"].Bind( wx.EVT_CHECKBOX, self.dialog._on_taskbar_icon_text_enabled_changed, ) - sizer.Add(controls["taskbar_icon_text_enabled"], 0, wx.ALL, 5) + tray_section.Add( + controls["taskbar_icon_text_enabled"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) controls["taskbar_icon_dynamic_enabled"] = wx.CheckBox( - panel, label="Update tray text dynamically" + panel, + label="Update tray text as conditions change", ) - sizer.Add(controls["taskbar_icon_dynamic_enabled"], 0, wx.LEFT | wx.BOTTOM, 15) - - row_taskbar_format = wx.BoxSizer(wx.HORIZONTAL) - row_taskbar_format.Add( - wx.StaticText(panel, label="Current tray text format:"), + tray_section.Add( + controls["taskbar_icon_dynamic_enabled"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) + controls["taskbar_icon_text_format"] = wx.TextCtrl( panel, - size=(280, -1), + size=(320, -1), style=wx.TE_READONLY, ) - row_taskbar_format.Add(controls["taskbar_icon_text_format"], 1) - controls["taskbar_icon_text_format_dialog"] = wx.Button(panel, label="Edit Format...") + self.dialog.add_labeled_row( + panel, + tray_section, + "Current tray text format:", + controls["taskbar_icon_text_format"], + expand_control=True, + ) + controls["taskbar_icon_text_format_dialog"] = wx.Button( + panel, + label="Edit tray text format...", + ) controls["taskbar_icon_text_format_dialog"].Bind( wx.EVT_BUTTON, self.dialog._on_edit_taskbar_text_format, ) - row_taskbar_format.Add(controls["taskbar_icon_text_format_dialog"], 0, wx.LEFT, 8) - sizer.Add(row_taskbar_format, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 15) + tray_section.Add( + controls["taskbar_icon_text_format_dialog"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM, + 10, + ) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "General") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -89,7 +129,6 @@ def load(self, settings): controls["update_interval"].SetValue(getattr(settings, "update_interval_minutes", 10)) - # Show Nationwide — also gated on data source (cross-tab dep handled by dialog) data_source = getattr(settings, "data_source", "auto") if data_source not in ("auto", "nws"): controls["show_nationwide"].SetValue(False) @@ -125,10 +164,10 @@ def setup_accessibility(self): """Set accessibility names for General tab controls.""" controls = self.dialog._controls names = { - "update_interval": "Update Interval (minutes)", - "show_nationwide": "Show Nationwide location (requires Auto or NWS data source)", - "taskbar_icon_text_enabled": "Show weather text on tray icon", - "taskbar_icon_dynamic_enabled": "Update tray text dynamically", + "update_interval": "Update interval in minutes", + "show_nationwide": "Show the Nationwide location when a supported data source is selected", + "taskbar_icon_text_enabled": "Show weather text on the tray icon", + "taskbar_icon_dynamic_enabled": "Update tray text as conditions change", "taskbar_icon_text_format": "Tray text format", } for key, name in names.items(): diff --git a/src/accessiweather/ui/dialogs/settings_tabs/notifications.py b/src/accessiweather/ui/dialogs/settings_tabs/notifications.py index 28493681..215a6414 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/notifications.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/notifications.py @@ -19,112 +19,135 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "Alerts"): """Build the Notifications tab panel and add it to the notebook.""" panel = wx.ScrolledWindow(self.dialog.notebook) panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - sizer.Add(wx.StaticText(panel, label="Alert Notification Settings"), 0, wx.ALL, 5) - sizer.Add( - wx.StaticText( - panel, - label="Configure which weather alerts trigger notifications based on severity.", - ), - 0, - wx.LEFT | wx.BOTTOM, - 5, + self.dialog.add_help_text( + panel, + sizer, + "Control which alerts notify you, how broad the alert area is, and how aggressively notifications repeat.", + left=5, ) - controls["enable_alerts"] = wx.CheckBox(panel, label="Enable weather alerts") - sizer.Add(controls["enable_alerts"], 0, wx.ALL, 5) - - controls["alert_notif"] = wx.CheckBox(panel, label="Enable alert notifications") - sizer.Add(controls["alert_notif"], 0, wx.LEFT | wx.BOTTOM, 5) - + delivery_section = self.dialog.create_section( + panel, + sizer, + "Alert delivery", + "Turn alert monitoring on or off, then choose whether AccessiWeather should notify you when alerts arrive.", + ) + controls["enable_alerts"] = wx.CheckBox(panel, label="Monitor weather alerts") + delivery_section.Add(controls["enable_alerts"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + controls["alert_notif"] = wx.CheckBox(panel, label="Send alert notifications") + delivery_section.Add(controls["alert_notif"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["immediate_alert_details_popups"] = wx.CheckBox( panel, - label="Open alert details popups immediately while AccessiWeather is running", + label="Open alert details immediately while AccessiWeather is running", ) - sizer.Add(controls["immediate_alert_details_popups"], 0, wx.LEFT | wx.BOTTOM, 5) - - row_area = wx.BoxSizer(wx.HORIZONTAL) - row_area.Add( - wx.StaticText(panel, label="Alert Area:"), + delivery_section.Add( + controls["immediate_alert_details_popups"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) + + coverage_section = self.dialog.create_section( + panel, + sizer, + "Coverage and severity", + "Smaller alert areas are quieter. Broader areas catch more alerts but can be noisier.", + ) controls["alert_radius_type"] = wx.Choice( panel, choices=[ - "County (recommended — alerts for your county only)", - "Point (exact coordinate — may miss alerts)", - "Zone (NWS forecast zone — slightly broader than county)", - "State (entire state — noisy)", + "County (recommended)", + "Point (exact coordinate, may miss alerts)", + "Zone (slightly broader than county)", + "State (broadest and noisiest)", ], ) - row_area.Add(controls["alert_radius_type"], 0) - sizer.Add(row_area, 0, wx.ALL, 5) - - sizer.Add(wx.StaticText(panel, label="Alert Severity Levels:"), 0, wx.ALL, 5) - + self.dialog.add_labeled_row( + panel, + coverage_section, + "Alert area:", + controls["alert_radius_type"], + ) controls["notify_extreme"] = wx.CheckBox( - panel, label="Extreme - Life-threatening events (e.g., Tornado Warning)" + panel, + label="Extreme severity alerts", ) - sizer.Add(controls["notify_extreme"], 0, wx.LEFT, 10) - + coverage_section.Add(controls["notify_extreme"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["notify_severe"] = wx.CheckBox( - panel, label="Severe - Significant hazards (e.g., Severe Thunderstorm Warning)" + panel, + label="Severe severity alerts", ) - sizer.Add(controls["notify_severe"], 0, wx.LEFT, 10) - + coverage_section.Add(controls["notify_severe"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["notify_moderate"] = wx.CheckBox( - panel, label="Moderate - Potentially hazardous (e.g., Winter Weather Advisory)" + panel, + label="Moderate severity alerts", ) - sizer.Add(controls["notify_moderate"], 0, wx.LEFT, 10) - + coverage_section.Add(controls["notify_moderate"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) controls["notify_minor"] = wx.CheckBox( - panel, label="Minor - Low impact events (e.g., Frost Advisory, Fog Advisory)" + panel, + label="Minor severity alerts", ) - sizer.Add(controls["notify_minor"], 0, wx.LEFT, 10) - - controls["notify_unknown"] = wx.CheckBox(panel, label="Unknown - Uncategorized alerts") - sizer.Add(controls["notify_unknown"], 0, wx.LEFT, 10) - - sizer.Add(wx.StaticText(panel, label="Event-Based Notifications:"), 0, wx.ALL, 5) - sizer.Add( - wx.StaticText( - panel, - label="Get notified when specific weather events occur (disabled by default).", - ), - 0, - wx.LEFT | wx.BOTTOM, - 5, + coverage_section.Add(controls["notify_minor"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + controls["notify_unknown"] = wx.CheckBox( + panel, + label="Uncategorized alerts", ) + coverage_section.Add(controls["notify_unknown"], 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + event_section = self.dialog.create_section( + panel, + sizer, + "Extra weather event notifications", + "These are optional updates beyond standard alerts and are off unless you turn them on.", + ) controls["notify_discussion_update"] = wx.CheckBox( - panel, label="Notify when Area Forecast Discussion is updated (NWS US only)" + panel, + label="Notify when the Area Forecast Discussion changes (NWS US only)", + ) + event_section.Add( + controls["notify_discussion_update"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["notify_discussion_update"], 0, wx.LEFT, 10) - controls["notify_severe_risk_change"] = wx.CheckBox( - panel, label="Notify when severe weather risk level changes (Visual Crossing only)" + panel, + label="Notify when severe weather risk changes (Visual Crossing)", + ) + event_section.Add( + controls["notify_severe_risk_change"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["notify_severe_risk_change"], 0, wx.LEFT | wx.BOTTOM, 10) - controls["notify_minutely_precipitation_start"] = wx.CheckBox( - panel, label="Notify when precipitation is expected to start soon (Pirate Weather)" + panel, + label="Notify when precipitation is expected to start soon (Pirate Weather)", + ) + event_section.Add( + controls["notify_minutely_precipitation_start"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["notify_minutely_precipitation_start"], 0, wx.LEFT, 10) - controls["notify_minutely_precipitation_stop"] = wx.CheckBox( - panel, label="Notify when precipitation is expected to stop soon (Pirate Weather)" + panel, + label="Notify when precipitation is expected to stop soon (Pirate Weather)", + ) + event_section.Add( + controls["notify_minutely_precipitation_stop"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, ) - sizer.Add(controls["notify_minutely_precipitation_stop"], 0, wx.LEFT | wx.BOTTOM, 10) - # Hidden alert timing controls (values managed via Advanced dialog) controls["global_cooldown"] = wx.SpinCtrl(panel, min=0, max=60, initial=5) controls["global_cooldown"].Hide() controls["per_alert_cooldown"] = wx.SpinCtrl(panel, min=0, max=1440, initial=60) @@ -132,27 +155,27 @@ def create(self): controls["freshness_window"] = wx.SpinCtrl(panel, min=0, max=120, initial=15) controls["freshness_window"].Hide() - sizer.Add(wx.StaticText(panel, label="Rate Limiting:"), 0, wx.ALL, 5) - - row_max = wx.BoxSizer(wx.HORIZONTAL) - row_max.Add( - wx.StaticText(panel, label="Maximum notifications per hour:"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + rate_section = self.dialog.create_section( + panel, + sizer, + "Rate limiting", + "Use the advanced timing dialog if you need cooldown and freshness controls beyond the hourly limit.", ) controls["max_notifications"] = wx.SpinCtrl(panel, min=1, max=100, initial=10) - row_max.Add(controls["max_notifications"], 0) - sizer.Add(row_max, 0, wx.LEFT | wx.TOP, 10) - - advanced_btn = wx.Button(panel, label="Advanced...") + self.dialog.add_labeled_row( + panel, + rate_section, + "Maximum notifications per hour:", + controls["max_notifications"], + ) + advanced_btn = wx.Button(panel, label="Advanced timing...") advanced_btn.SetName("Advanced alert timing settings") - advanced_btn.SetToolTip("Configure cooldown periods and alert freshness window") + advanced_btn.SetToolTip("Configure cooldown periods and the alert freshness window") advanced_btn.Bind(wx.EVT_BUTTON, self.dialog._on_alert_advanced) - sizer.Add(advanced_btn, 0, wx.LEFT | wx.TOP, 10) + rate_section.Add(advanced_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Notifications") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -229,22 +252,22 @@ def setup_accessibility(self): """Set accessibility names for Notifications tab controls.""" controls = self.dialog._controls names = { - "enable_alerts": "Enable weather alerts", - "alert_notif": "Enable alert notifications", - "alert_radius_type": "Alert Area", - "notify_extreme": "Extreme - Life-threatening events (e.g., Tornado Warning)", - "notify_severe": "Severe - Significant hazards (e.g., Severe Thunderstorm Warning)", - "notify_moderate": "Moderate - Potentially hazardous (e.g., Winter Weather Advisory)", - "notify_minor": "Minor - Low impact events (e.g., Frost Advisory, Fog Advisory)", - "notify_unknown": "Unknown - Uncategorized alerts", - "immediate_alert_details_popups": "Open alert details popups immediately while AccessiWeather is running", - "notify_discussion_update": "Notify when Area Forecast Discussion is updated (NWS US only)", - "notify_severe_risk_change": "Notify when severe weather risk level changes (Visual Crossing only)", - "notify_minutely_precipitation_start": "Notify when precipitation is expected to start soon (Pirate Weather)", - "notify_minutely_precipitation_stop": "Notify when precipitation is expected to stop soon (Pirate Weather)", - "global_cooldown": "Minimum time between any alert notifications (minutes)", - "per_alert_cooldown": "Re-notify for same alert after (minutes)", - "freshness_window": "Only notify for alerts issued within (minutes)", + "enable_alerts": "Monitor weather alerts", + "alert_notif": "Send alert notifications", + "alert_radius_type": "Alert area", + "notify_extreme": "Extreme severity alerts", + "notify_severe": "Severe severity alerts", + "notify_moderate": "Moderate severity alerts", + "notify_minor": "Minor severity alerts", + "notify_unknown": "Uncategorized alerts", + "immediate_alert_details_popups": "Open alert details immediately while AccessiWeather is running", + "notify_discussion_update": "Notify when the Area Forecast Discussion changes", + "notify_severe_risk_change": "Notify when severe weather risk changes", + "notify_minutely_precipitation_start": "Notify when precipitation is expected to start soon", + "notify_minutely_precipitation_stop": "Notify when precipitation is expected to stop soon", + "global_cooldown": "Minimum time between any alert notifications in minutes", + "per_alert_cooldown": "Minutes before repeating the same alert notification", + "freshness_window": "Only notify for alerts issued within this many minutes", "max_notifications": "Maximum notifications per hour", } for key, name in names.items(): diff --git a/src/accessiweather/ui/dialogs/settings_tabs/updates.py b/src/accessiweather/ui/dialogs/settings_tabs/updates.py index 479b26c1..20f0f00c 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/updates.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/updates.py @@ -16,52 +16,76 @@ def __init__(self, dialog): """Store reference to the parent settings dialog.""" self.dialog = dialog - def create(self): + def create(self, page_label: str = "Updates"): """Build the Updates tab panel and add it to the notebook.""" - panel = wx.Panel(self.dialog.notebook) + panel = wx.ScrolledWindow(self.dialog.notebook) + panel.SetScrollRate(0, 20) sizer = wx.BoxSizer(wx.VERTICAL) controls = self.dialog._controls - controls["auto_update"] = wx.CheckBox(panel, label="Check for updates automatically") - sizer.Add(controls["auto_update"], 0, wx.ALL, 5) + self.dialog.add_help_text( + panel, + sizer, + "Choose how AccessiWeather checks for new releases and when you want to check manually.", + left=5, + ) - row_ch = wx.BoxSizer(wx.HORIZONTAL) - row_ch.Add( - wx.StaticText(panel, label="Update Channel:"), + auto_section = self.dialog.create_section( + panel, + sizer, + "Automatic update checks", + "Stable is safest for everyday use. Development includes the latest work but may be less predictable.", + ) + controls["auto_update"] = wx.CheckBox(panel, label="Check for updates automatically") + auto_section.Add( + controls["auto_update"], 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) controls["update_channel"] = wx.Choice( panel, choices=[ - "Stable (Production releases only)", - "Development (Latest features, may be unstable)", + "Stable (production releases only)", + "Development (latest features, may be unstable)", ], ) - row_ch.Add(controls["update_channel"], 0) - sizer.Add(row_ch, 0, wx.LEFT, 5) - - row_int = wx.BoxSizer(wx.HORIZONTAL) - row_int.Add( - wx.StaticText(panel, label="Check Interval (hours):"), - 0, - wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, - 10, + self.dialog.add_labeled_row( + panel, + auto_section, + "Release channel:", + controls["update_channel"], ) controls["update_check_interval"] = wx.SpinCtrl(panel, min=1, max=168, initial=24) - row_int.Add(controls["update_check_interval"], 0) - sizer.Add(row_int, 0, wx.LEFT | wx.TOP, 5) + self.dialog.add_labeled_row( + panel, + auto_section, + "Check every (hours):", + controls["update_check_interval"], + ) - check_btn = wx.Button(panel, label="Check for Updates Now") + manual_section = self.dialog.create_section( + panel, + sizer, + "Check now", + "Use this if you want an immediate update check instead of waiting for the next scheduled one.", + ) + check_btn = wx.Button(panel, label="Check for updates now") check_btn.Bind(wx.EVT_BUTTON, self.dialog._on_check_updates) - sizer.Add(check_btn, 0, wx.ALL, 10) - - controls["update_status"] = wx.StaticText(panel, label="Ready to check for updates") - sizer.Add(controls["update_status"], 0, wx.LEFT, 10) + manual_section.Add(check_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) + controls["update_status"] = wx.StaticText( + panel, + label="Ready to check for updates.", + ) + manual_section.Add( + controls["update_status"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) panel.SetSizer(sizer) - self.dialog.notebook.AddPage(panel, "Updates") + self.dialog.notebook.AddPage(panel, page_label) return panel def load(self, settings): @@ -88,8 +112,8 @@ def setup_accessibility(self): controls = self.dialog._controls names = { "auto_update": "Check for updates automatically", - "update_channel": "Update Channel", - "update_check_interval": "Check Interval (hours)", + "update_channel": "Release channel", + "update_check_interval": "Update check interval in hours", } for key, name in names.items(): controls[key].SetName(name) diff --git a/tests/test_settings_dialog_audio_events.py b/tests/test_settings_dialog_audio_events.py index 1e22dce7..0423129a 100644 --- a/tests/test_settings_dialog_audio_events.py +++ b/tests/test_settings_dialog_audio_events.py @@ -104,7 +104,7 @@ def test_load_settings_updates_event_sound_state_and_summary(): total_events = len(AudioTab._build_default_event_sound_states()) assert ( dialog._controls["event_sounds_summary"].GetLabel() - == f"{total_events - 1} of {total_events} sound events are enabled." + == f"Sounds will play for {total_events - 1} of {total_events} selectable event types." ) @@ -142,7 +142,7 @@ def test_configure_event_sounds_applies_modal_result_and_refreshes_summary(): total_events = len(AudioTab._build_default_event_sound_states()) assert ( dialog._controls["event_sounds_summary"].GetLabel() - == f"{total_events - 2} of {total_events} sound events are enabled." + == f"Sounds will play for {total_events - 2} of {total_events} selectable event types." ) diff --git a/tests/test_settings_dialog_ux_metadata.py b/tests/test_settings_dialog_ux_metadata.py new file mode 100644 index 00000000..66851ee4 --- /dev/null +++ b/tests/test_settings_dialog_ux_metadata.py @@ -0,0 +1,54 @@ +from __future__ import annotations + +from accessiweather.ui.dialogs.settings_dialog import SettingsDialogSimple +from accessiweather.ui.dialogs.settings_tabs.audio import AudioTab +from accessiweather.ui.dialogs.settings_tabs.data_sources import DataSourcesTab + + +def test_dialog_tab_definitions_follow_user_focused_order_and_labels(): + assert SettingsDialogSimple.get_tab_definitions() == [ + ("general", "General"), + ("display", "Display"), + ("notifications", "Alerts"), + ("audio", "Audio"), + ("data_sources", "Data Sources"), + ("ai", "AI"), + ("updates", "Updates"), + ("advanced", "Advanced"), + ] + + +def test_audio_event_sound_summary_mentions_sound_choices_clearly(): + enabled = AudioTab._build_default_event_sound_states() + total = len(enabled) + assert AudioTab.build_event_sound_summary_text(enabled) == ( + f"Sounds will play for all {total} selectable event types." + ) + + some_disabled = dict(enabled) + some_disabled["data_updated"] = False + some_disabled["fetch_error"] = False + assert AudioTab.build_event_sound_summary_text(some_disabled) == ( + f"Sounds will play for {total - 2} of {total} selectable event types." + ) + + all_disabled = dict.fromkeys(enabled, False) + assert ( + AudioTab.build_event_sound_summary_text(all_disabled) + == "Sounds are turned off for every selectable event type." + ) + + +def test_source_settings_summary_uses_plain_language(): + state = { + "auto_use_nws": True, + "auto_use_openmeteo": True, + "auto_use_visualcrossing": False, + "auto_use_pirateweather": True, + "station_selection_strategy": 2, + } + + assert DataSourcesTab.build_source_settings_summary_text(state) == ( + "Automatic mode uses: NWS, Open-Meteo, Pirate Weather. " + "NWS station strategy: Major airport preferred." + ) From 20dcf041373427edbbd24c0c6cf8e542539cdc65 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:22:48 +0000 Subject: [PATCH 2/8] fix(a11y): keep section help text from preceding controls --- .../ui/dialogs/settings_dialog.py | 11 +++++-- tests/test_settings_dialog_ux_metadata.py | 31 +++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_dialog.py b/src/accessiweather/ui/dialogs/settings_dialog.py index ae345b4d..ef623673 100644 --- a/src/accessiweather/ui/dialogs/settings_dialog.py +++ b/src/accessiweather/ui/dialogs/settings_dialog.py @@ -209,10 +209,15 @@ def create_section( title: str, description: str | None = None, ) -> wx.StaticBoxSizer: - """Create a titled settings section with optional helper copy.""" + """ + Create a titled settings section. + + The description parameter is accepted for call-site readability, but we do + not auto-render it above the first interactive control. Doing so can cause + screen readers to announce helper copy before the actual control label. + """ + del description section = wx.StaticBoxSizer(wx.VERTICAL, parent, title) - if description: - self.add_help_text(parent, section, description) parent_sizer.Add(section, 0, wx.EXPAND | wx.ALL, 5) return section diff --git a/tests/test_settings_dialog_ux_metadata.py b/tests/test_settings_dialog_ux_metadata.py index 66851ee4..d64f9bc6 100644 --- a/tests/test_settings_dialog_ux_metadata.py +++ b/tests/test_settings_dialog_ux_metadata.py @@ -1,5 +1,8 @@ from __future__ import annotations +from unittest.mock import MagicMock + +import accessiweather.ui.dialogs.settings_dialog as settings_module from accessiweather.ui.dialogs.settings_dialog import SettingsDialogSimple from accessiweather.ui.dialogs.settings_tabs.audio import AudioTab from accessiweather.ui.dialogs.settings_tabs.data_sources import DataSourcesTab @@ -52,3 +55,31 @@ def test_source_settings_summary_uses_plain_language(): "Automatic mode uses: NWS, Open-Meteo, Pirate Weather. " "NWS station strategy: Major airport preferred." ) + + +def test_create_section_does_not_insert_helper_text_before_controls(monkeypatch): + fake_section = MagicMock() + monkeypatch.setattr( + settings_module.wx, + "StaticBoxSizer", + MagicMock(return_value=fake_section), + raising=False, + ) + monkeypatch.setattr(settings_module.wx, "VERTICAL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "EXPAND", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "ALL", 0, raising=False) + + dialog = SettingsDialogSimple.__new__(SettingsDialogSimple) + dialog.add_help_text = MagicMock() + parent_sizer = MagicMock() + + section = dialog.create_section( + parent=MagicMock(), + parent_sizer=parent_sizer, + title="Weather refresh", + description="This should not be injected before the first control.", + ) + + assert section is fake_section + dialog.add_help_text.assert_not_called() + parent_sizer.Add.assert_called_once() From 2ad3c23ce5995969f70d8b300ab5cf2846535f9a Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:28:39 +0000 Subject: [PATCH 3/8] fix(ui): separate display units from forecast length --- .../ui/dialogs/settings_tabs/display.py | 39 +++++++++++-------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index 553e4547..947aab27 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -41,11 +41,11 @@ def create(self, page_label: str = "Display"): left=5, ) - units_section = self.dialog.create_section( + temperature_section = self.dialog.create_section( panel, sizer, - "Units and forecast length", - "These settings control the overall scale and amount of forecast information you see.", + "Temperature units", + "Choose whether temperatures are shown automatically, in Fahrenheit, in Celsius, or both.", ) controls["temp_unit"] = wx.Choice( panel, @@ -58,37 +58,44 @@ def create(self, page_label: str = "Display"): ) self.dialog.add_labeled_row( panel, - units_section, + temperature_section, "Temperature units:", controls["temp_unit"], ) + controls["round_values"] = wx.CheckBox( + panel, + label="Show values as whole numbers when possible", + ) + temperature_section.Add( + controls["round_values"], + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 10, + ) + + forecast_section = self.dialog.create_section( + panel, + sizer, + "Forecast length", + "Choose how many days and hours of forecast detail AccessiWeather should show.", + ) controls["forecast_duration_days"] = wx.Choice( panel, choices=["3 days", "5 days", "7 days (default)", "10 days", "14 days", "15 days"], ) self.dialog.add_labeled_row( panel, - units_section, + forecast_section, "Forecast length:", controls["forecast_duration_days"], ) controls["hourly_forecast_hours"] = wx.SpinCtrl(panel, min=1, max=168, initial=6) self.dialog.add_labeled_row( panel, - units_section, + forecast_section, "Hourly forecast hours:", controls["hourly_forecast_hours"], ) - controls["round_values"] = wx.CheckBox( - panel, - label="Show values as whole numbers when possible", - ) - units_section.Add( - controls["round_values"], - 0, - wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, - 10, - ) details_section = self.dialog.create_section( panel, From 59ce78b6d381f8c2bdae9835a749688d672b9e24 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:32:04 +0000 Subject: [PATCH 4/8] fix(ui): clarify hourly forecast duration label --- src/accessiweather/ui/dialogs/settings_tabs/display.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index 947aab27..bbe38b73 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -93,7 +93,7 @@ def create(self, page_label: str = "Display"): self.dialog.add_labeled_row( panel, forecast_section, - "Hourly forecast hours:", + "Show hourly forecast for (hours):", controls["hourly_forecast_hours"], ) @@ -293,7 +293,7 @@ def setup_accessibility(self): "show_impact_summaries": "Show impact summaries for outdoor driving and allergy conditions", "round_values": "Show values as whole numbers when possible", "forecast_duration_days": "Forecast length", - "hourly_forecast_hours": "Hourly forecast hours", + "hourly_forecast_hours": "Show hourly forecast for hours", "forecast_time_reference": "Forecast time reference", "time_display_mode": "Time display mode", "time_format_12hour": "Use 12-hour time format", From db9969af4466d65498fde6bb8ab3f4717a0f72ca Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:33:59 +0000 Subject: [PATCH 5/8] fix(ui): clarify daily and hourly forecast range labels --- src/accessiweather/ui/dialogs/settings_tabs/display.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index bbe38b73..72c2e093 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -76,7 +76,7 @@ def create(self, page_label: str = "Display"): forecast_section = self.dialog.create_section( panel, sizer, - "Forecast length", + "Forecast range", "Choose how many days and hours of forecast detail AccessiWeather should show.", ) controls["forecast_duration_days"] = wx.Choice( @@ -86,14 +86,14 @@ def create(self, page_label: str = "Display"): self.dialog.add_labeled_row( panel, forecast_section, - "Forecast length:", + "Daily forecast range:", controls["forecast_duration_days"], ) controls["hourly_forecast_hours"] = wx.SpinCtrl(panel, min=1, max=168, initial=6) self.dialog.add_labeled_row( panel, forecast_section, - "Show hourly forecast for (hours):", + "Hourly forecast range (hours):", controls["hourly_forecast_hours"], ) @@ -292,8 +292,8 @@ def setup_accessibility(self): "show_pressure_trend": "Show pressure trend", "show_impact_summaries": "Show impact summaries for outdoor driving and allergy conditions", "round_values": "Show values as whole numbers when possible", - "forecast_duration_days": "Forecast length", - "hourly_forecast_hours": "Show hourly forecast for hours", + "forecast_duration_days": "Daily forecast range", + "hourly_forecast_hours": "Hourly forecast range in hours", "forecast_time_reference": "Forecast time reference", "time_display_mode": "Time display mode", "time_format_12hour": "Use 12-hour time format", From 8db9de67c87e205b84e8f19f5c2d120ca93b16bf Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:50:22 +0000 Subject: [PATCH 6/8] fix(a11y): replace settings static boxes with headings --- .../ui/dialogs/settings_dialog.py | 17 ++++++++----- tests/test_settings_dialog_ux_metadata.py | 24 +++++++++++++++---- 2 files changed, 30 insertions(+), 11 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_dialog.py b/src/accessiweather/ui/dialogs/settings_dialog.py index ef623673..38b91df9 100644 --- a/src/accessiweather/ui/dialogs/settings_dialog.py +++ b/src/accessiweather/ui/dialogs/settings_dialog.py @@ -208,17 +208,22 @@ def create_section( parent_sizer: wx.Sizer, title: str, description: str | None = None, - ) -> wx.StaticBoxSizer: + ) -> wx.BoxSizer: """ Create a titled settings section. - The description parameter is accepted for call-site readability, but we do - not auto-render it above the first interactive control. Doing so can cause - screen readers to announce helper copy before the actual control label. + We intentionally avoid StaticBoxSizer here because screen readers can + announce the group label as part of the first interactive control in the + section. The description parameter is accepted for call-site readability, + but we do not auto-render it above the first interactive control. """ del description - section = wx.StaticBoxSizer(wx.VERTICAL, parent, title) - parent_sizer.Add(section, 0, wx.EXPAND | wx.ALL, 5) + heading = wx.StaticText(parent, label=title) + self._wrap_static_text(heading) + parent_sizer.Add(heading, 0, wx.LEFT | wx.RIGHT | wx.TOP | wx.EXPAND, 5) + + section = wx.BoxSizer(wx.VERTICAL) + parent_sizer.Add(section, 0, wx.EXPAND | wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) return section def _create_ui(self): diff --git a/tests/test_settings_dialog_ux_metadata.py b/tests/test_settings_dialog_ux_metadata.py index d64f9bc6..2a861e66 100644 --- a/tests/test_settings_dialog_ux_metadata.py +++ b/tests/test_settings_dialog_ux_metadata.py @@ -57,29 +57,43 @@ def test_source_settings_summary_uses_plain_language(): ) -def test_create_section_does_not_insert_helper_text_before_controls(monkeypatch): +def test_create_section_uses_heading_and_plain_sizer_for_accessibility(monkeypatch): fake_section = MagicMock() + fake_heading = MagicMock() monkeypatch.setattr( settings_module.wx, - "StaticBoxSizer", + "BoxSizer", MagicMock(return_value=fake_section), raising=False, ) + monkeypatch.setattr( + settings_module.wx, + "StaticText", + MagicMock(return_value=fake_heading), + raising=False, + ) monkeypatch.setattr(settings_module.wx, "VERTICAL", 0, raising=False) monkeypatch.setattr(settings_module.wx, "EXPAND", 0, raising=False) - monkeypatch.setattr(settings_module.wx, "ALL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "LEFT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "RIGHT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "TOP", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "BOTTOM", 0, raising=False) dialog = SettingsDialogSimple.__new__(SettingsDialogSimple) dialog.add_help_text = MagicMock() + dialog._wrap_static_text = MagicMock() parent_sizer = MagicMock() + parent = MagicMock() section = dialog.create_section( - parent=MagicMock(), + parent=parent, parent_sizer=parent_sizer, title="Weather refresh", description="This should not be injected before the first control.", ) assert section is fake_section + settings_module.wx.StaticText.assert_called_once_with(parent, label="Weather refresh") + dialog._wrap_static_text.assert_called_once_with(fake_heading) dialog.add_help_text.assert_not_called() - parent_sizer.Add.assert_called_once() + assert parent_sizer.Add.call_count == 2 From 770291de5f8fe1b651e74e7a3df8e69086584f15 Mon Sep 17 00:00:00 2001 From: Orinks Date: Wed, 8 Apr 2026 23:53:47 +0000 Subject: [PATCH 7/8] fix(a11y): create forecast labels before controls --- .../ui/dialogs/settings_tabs/display.py | 37 ++++++++++++++----- 1 file changed, 27 insertions(+), 10 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index 72c2e093..ea042962 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -79,22 +79,39 @@ def create(self, page_label: str = "Display"): "Forecast range", "Choose how many days and hours of forecast detail AccessiWeather should show.", ) + daily_forecast_row = wx.BoxSizer(wx.HORIZONTAL) + daily_forecast_row.Add( + wx.StaticText(panel, label="Daily forecast range:"), + 0, + wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + 10, + ) controls["forecast_duration_days"] = wx.Choice( panel, choices=["3 days", "5 days", "7 days (default)", "10 days", "14 days", "15 days"], ) - self.dialog.add_labeled_row( - panel, - forecast_section, - "Daily forecast range:", - controls["forecast_duration_days"], + daily_forecast_row.Add(controls["forecast_duration_days"], 0) + forecast_section.Add( + daily_forecast_row, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 8, + ) + + hourly_forecast_row = wx.BoxSizer(wx.HORIZONTAL) + hourly_forecast_row.Add( + wx.StaticText(panel, label="Hourly forecast range (hours):"), + 0, + wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + 10, ) controls["hourly_forecast_hours"] = wx.SpinCtrl(panel, min=1, max=168, initial=6) - self.dialog.add_labeled_row( - panel, - forecast_section, - "Hourly forecast range (hours):", - controls["hourly_forecast_hours"], + hourly_forecast_row.Add(controls["hourly_forecast_hours"], 0) + forecast_section.Add( + hourly_forecast_row, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 8, ) details_section = self.dialog.create_section( From 743429a63da75334dba8f60ee4b467c1cad16217 Mon Sep 17 00:00:00 2001 From: Orinks Date: Thu, 9 Apr 2026 00:14:24 +0000 Subject: [PATCH 8/8] fix(a11y): stabilize settings dialog label associations --- .../ui/dialogs/settings_dialog.py | 65 ++++++--- .../ui/dialogs/settings_tabs/ai.py | 43 +++--- .../ui/dialogs/settings_tabs/audio.py | 5 +- .../ui/dialogs/settings_tabs/data_sources.py | 75 +++++------ .../ui/dialogs/settings_tabs/display.py | 62 ++++----- .../ui/dialogs/settings_tabs/general.py | 18 ++- .../ui/dialogs/settings_tabs/notifications.py | 26 ++-- .../ui/dialogs/settings_tabs/updates.py | 22 ++- tests/test_dialog_accessibility.py | 9 +- tests/test_settings_dialog_ux_metadata.py | 127 ++++++++++++++++++ 10 files changed, 299 insertions(+), 153 deletions(-) diff --git a/src/accessiweather/ui/dialogs/settings_dialog.py b/src/accessiweather/ui/dialogs/settings_dialog.py index 38b91df9..2d4c3516 100644 --- a/src/accessiweather/ui/dialogs/settings_dialog.py +++ b/src/accessiweather/ui/dialogs/settings_dialog.py @@ -202,6 +202,29 @@ def add_labeled_row( parent_sizer.Add(row, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, bottom) return row + def add_labeled_control_row( + self, + parent: wx.Window, + parent_sizer: wx.Sizer, + label: str, + control_factory, + *, + expand_control: bool = False, + bottom: int = 8, + ) -> wx.Window: + """Create the visible label before the control for wx/NVDA association stability.""" + row = wx.BoxSizer(wx.HORIZONTAL) + row.Add( + wx.StaticText(parent, label=label), + 0, + wx.ALIGN_CENTER_VERTICAL | wx.RIGHT, + 10, + ) + control = control_factory(parent) + row.Add(control, 1 if expand_control else 0, wx.EXPAND if expand_control else 0) + parent_sizer.Add(row, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, bottom) + return control + def create_section( self, parent: wx.Window, @@ -445,9 +468,11 @@ def _run_event_sounds_dialog(self) -> dict[str, bool] | None: scroll_sizer = wx.BoxSizer(wx.VERTICAL) for section_title, description, event_keys in self._get_event_sound_sections(): - section = wx.StaticBoxSizer(wx.VERTICAL, scroll, section_title) + section = wx.BoxSizer(wx.VERTICAL) + heading = wx.StaticText(scroll, label=section_title) + self._wrap_static_text(heading, width=380) section.Add( - wx.StaticText(scroll, label=description), + heading, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 5, @@ -457,6 +482,14 @@ def _run_event_sounds_dialog(self) -> dict[str, bool] | None: checkbox.SetValue(state_map.get(event_key, True)) dialog_controls[event_key] = checkbox section.Add(checkbox, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 5) + description_text = wx.StaticText(scroll, label=description) + self._wrap_static_text(description_text, width=380) + section.Add( + description_text, + 0, + wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, + 5, + ) scroll_sizer.Add(section, 0, wx.ALL | wx.EXPAND, 5) scroll.SetSizer(scroll_sizer) @@ -523,23 +556,23 @@ def _run_source_settings_dialog(self) -> dict | None: 10, ) - cc_sizer.Add( - wx.StaticText(cc_panel, label="NWS station selection strategy:"), - 0, - wx.LEFT | wx.RIGHT, - 10, - ) - strategy_ctrl = wx.Choice( + strategy_ctrl = self.add_labeled_control_row( cc_panel, - choices=[ - "Hybrid default (recommended: fresh + major station with distance guardrail)", - "Nearest station (pure distance)", - "Major airport preferred (within radius, else nearest)", - "Freshest observation (among nearest stations)", - ], + cc_sizer, + "NWS station selection strategy:", + lambda parent: wx.Choice( + parent, + choices=[ + "Hybrid default (recommended: fresh + major station with distance guardrail)", + "Nearest station (pure distance)", + "Major airport preferred (within radius, else nearest)", + "Freshest observation (among nearest stations)", + ], + ), + expand_control=True, + bottom=10, ) strategy_ctrl.SetSelection(state.get("station_selection_strategy", 0)) - cc_sizer.Add(strategy_ctrl, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10) cc_panel.SetSizer(cc_sizer) notebook.AddPage(cc_panel, "Current Conditions") diff --git a/src/accessiweather/ui/dialogs/settings_tabs/ai.py b/src/accessiweather/ui/dialogs/settings_tabs/ai.py index 80553a9a..fda8f1e8 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/ai.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/ai.py @@ -39,12 +39,11 @@ def create(self, page_label: str = "AI"): "OpenRouter access", "Add and validate your OpenRouter API key before choosing a model.", ) - controls["openrouter_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(320, -1)) - self.dialog.add_labeled_row( + controls["openrouter_key"] = self.dialog.add_labeled_control_row( panel, key_section, "OpenRouter API key:", - controls["openrouter_key"], + lambda parent: wx.TextCtrl(parent, style=wx.TE_PASSWORD, size=(320, -1)), expand_control=True, ) validate_btn = wx.Button(panel, label="Validate OpenRouter key") @@ -57,36 +56,34 @@ def create(self, page_label: str = "AI"): "Model and explanation style", "Free options are easiest to start with. Paid routing can unlock more model choices.", ) - controls["ai_model"] = wx.Choice( - panel, - choices=[ - "Free router (automatic, free)", - "Llama 3.3 70B (free)", - "Auto router (paid)", - ], - ) - self.dialog.add_labeled_row( + controls["ai_model"] = self.dialog.add_labeled_control_row( panel, model_section, "Model preference:", - controls["ai_model"], + lambda parent: wx.Choice( + parent, + choices=[ + "Free router (automatic, free)", + "Llama 3.3 70B (free)", + "Auto router (paid)", + ], + ), ) browse_btn = wx.Button(panel, label="Browse OpenRouter models...") browse_btn.Bind(wx.EVT_BUTTON, self.dialog._on_browse_models) model_section.Add(browse_btn, 0, wx.LEFT | wx.RIGHT | wx.BOTTOM, 10) - controls["ai_style"] = wx.Choice( - panel, - choices=[ - "Brief (1-2 sentences)", - "Standard (3-4 sentences)", - "Detailed (full paragraph)", - ], - ) - self.dialog.add_labeled_row( + controls["ai_style"] = self.dialog.add_labeled_control_row( panel, model_section, "Explanation style:", - controls["ai_style"], + lambda parent: wx.Choice( + parent, + choices=[ + "Brief (1-2 sentences)", + "Standard (3-4 sentences)", + "Detailed (full paragraph)", + ], + ), ) prompt_section = self.dialog.create_section( diff --git a/src/accessiweather/ui/dialogs/settings_tabs/audio.py b/src/accessiweather/ui/dialogs/settings_tabs/audio.py index 63ea06ae..016aee8f 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/audio.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/audio.py @@ -129,12 +129,11 @@ def create(self, page_label: str = "Audio"): except Exception as e: logger.warning(f"Failed to load sound packs: {e}") - controls["sound_pack"] = wx.Choice(panel, choices=pack_names) - self.dialog.add_labeled_row( + controls["sound_pack"] = self.dialog.add_labeled_control_row( panel, playback_section, "Sound pack:", - controls["sound_pack"], + lambda parent: wx.Choice(parent, choices=pack_names), ) action_row = wx.BoxSizer(wx.HORIZONTAL) test_btn = wx.Button(panel, label="Play sample sound") diff --git a/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py b/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py index 08f497c0..7ffe7281 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/data_sources.py @@ -82,23 +82,22 @@ def create(self, page_label: str = "Data Sources"): "Choose a weather source", "Automatic mode combines available providers. Single-source options keep behavior predictable when you prefer one provider.", ) - controls["data_source"] = wx.Choice( - panel, - choices=[ - "Automatic (combine all available sources)", - "National Weather Service (US only, forecast and alerts)", - "Open-Meteo (global forecast, no alerts, no API key)", - "Visual Crossing (global forecast, regional alerts, API key)", - "Pirate Weather (global forecast and alerts, API key)", - ], - ) - controls["data_source"].Bind(wx.EVT_CHOICE, self.dialog._on_data_source_changed) - self.dialog.add_labeled_row( + controls["data_source"] = self.dialog.add_labeled_control_row( panel, source_section, "Weather source:", - controls["data_source"], + lambda parent: wx.Choice( + parent, + choices=[ + "Automatic (combine all available sources)", + "National Weather Service (US only, forecast and alerts)", + "Open-Meteo (global forecast, no alerts, no API key)", + "Visual Crossing (global forecast, regional alerts, API key)", + "Pirate Weather (global forecast and alerts, API key)", + ], + ), ) + controls["data_source"].Bind(wx.EVT_CHOICE, self.dialog._on_data_source_changed) auto_section = self.dialog.create_section( panel, @@ -140,28 +139,18 @@ def create(self, page_label: str = "Data Sources"): "Only Visual Crossing and Pirate Weather need keys. Stored keys stay in secure storage unless you explicitly export them.", ) - self.dialog._vc_config_sizer = wx.StaticBoxSizer( - wx.VERTICAL, - panel, - "Visual Crossing", - ) - vc_key_help = wx.StaticText( - panel, - label="Use this provider for global forecasts and regional alerts where available.", - ) - self.dialog._wrap_static_text(vc_key_help) + self.dialog._vc_config_sizer = wx.BoxSizer(wx.VERTICAL) self.dialog._vc_config_sizer.Add( - vc_key_help, + wx.StaticText(panel, label="Visual Crossing"), 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - controls["vc_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(280, -1)) - self.dialog.add_labeled_row( + controls["vc_key"] = self.dialog.add_labeled_control_row( panel, self.dialog._vc_config_sizer, "Visual Crossing API key:", - controls["vc_key"], + lambda parent: wx.TextCtrl(parent, style=wx.TE_PASSWORD, size=(280, -1)), expand_control=True, ) vc_button_row = wx.BoxSizer(wx.HORIZONTAL) @@ -177,30 +166,27 @@ def create(self, page_label: str = "Data Sources"): wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) - keys_section.Add(self.dialog._vc_config_sizer, 0, wx.EXPAND | wx.BOTTOM, 8) - - self.dialog._pw_config_sizer = wx.StaticBoxSizer( - wx.VERTICAL, - panel, - "Pirate Weather", - ) - pw_key_help = wx.StaticText( + self.dialog.add_help_text( panel, - label="Use this provider for global forecasts and broader alert coverage.", + self.dialog._vc_config_sizer, + "Use this provider for global forecasts and regional alerts where available.", + left=10, + bottom=10, ) - self.dialog._wrap_static_text(pw_key_help) + keys_section.Add(self.dialog._vc_config_sizer, 0, wx.EXPAND | wx.BOTTOM, 8) + + self.dialog._pw_config_sizer = wx.BoxSizer(wx.VERTICAL) self.dialog._pw_config_sizer.Add( - pw_key_help, + wx.StaticText(panel, label="Pirate Weather"), 0, wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - controls["pw_key"] = wx.TextCtrl(panel, style=wx.TE_PASSWORD, size=(280, -1)) - self.dialog.add_labeled_row( + controls["pw_key"] = self.dialog.add_labeled_control_row( panel, self.dialog._pw_config_sizer, "Pirate Weather API key:", - controls["pw_key"], + lambda parent: wx.TextCtrl(parent, style=wx.TE_PASSWORD, size=(280, -1)), expand_control=True, ) pw_button_row = wx.BoxSizer(wx.HORIZONTAL) @@ -216,6 +202,13 @@ def create(self, page_label: str = "Data Sources"): wx.LEFT | wx.RIGHT | wx.BOTTOM, 10, ) + self.dialog.add_help_text( + panel, + self.dialog._pw_config_sizer, + "Use this provider for global forecasts and broader alert coverage.", + left=10, + bottom=10, + ) keys_section.Add(self.dialog._pw_config_sizer, 0, wx.EXPAND) panel.SetSizer(sizer) diff --git a/src/accessiweather/ui/dialogs/settings_tabs/display.py b/src/accessiweather/ui/dialogs/settings_tabs/display.py index ea042962..5ee48279 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/display.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/display.py @@ -47,20 +47,19 @@ def create(self, page_label: str = "Display"): "Temperature units", "Choose whether temperatures are shown automatically, in Fahrenheit, in Celsius, or both.", ) - controls["temp_unit"] = wx.Choice( - panel, - choices=[ - "Auto (based on location)", - "Imperial (°F)", - "Metric (°C)", - "Both (°F and °C)", - ], - ) - self.dialog.add_labeled_row( + controls["temp_unit"] = self.dialog.add_labeled_control_row( panel, temperature_section, "Temperature units:", - controls["temp_unit"], + lambda parent: wx.Choice( + parent, + choices=[ + "Auto (based on location)", + "Imperial (°F)", + "Metric (°C)", + "Both (°F and °C)", + ], + ), ) controls["round_values"] = wx.CheckBox( panel, @@ -150,25 +149,23 @@ def create(self, page_label: str = "Display"): "Time display", "Choose whose timezone to use and how timestamps should be formatted.", ) - controls["forecast_time_reference"] = wx.Choice( - panel, - choices=["Location timezone (default)", "My local timezone"], - ) - self.dialog.add_labeled_row( + controls["forecast_time_reference"] = self.dialog.add_labeled_control_row( panel, time_section, "Forecast times are based on:", - controls["forecast_time_reference"], + lambda parent: wx.Choice( + parent, + choices=["Location timezone (default)", "My local timezone"], + ), ) - controls["time_display_mode"] = wx.Choice( - panel, - choices=["Local time only", "UTC time only", "Both local and UTC"], - ) - self.dialog.add_labeled_row( + controls["time_display_mode"] = self.dialog.add_labeled_control_row( panel, time_section, "Show times as:", - controls["time_display_mode"], + lambda parent: wx.Choice( + parent, + choices=["Local time only", "UTC time only", "Both local and UTC"], + ), ) controls["time_format_12hour"] = wx.CheckBox( panel, @@ -197,19 +194,18 @@ def create(self, page_label: str = "Display"): "Reading priority", "Decide how compact or detailed the spoken and displayed forecast should be.", ) - controls["verbosity_level"] = wx.Choice( - panel, - choices=[ - "Minimal (essentials only)", - "Standard (recommended)", - "Detailed (all available info)", - ], - ) - self.dialog.add_labeled_row( + controls["verbosity_level"] = self.dialog.add_labeled_control_row( panel, priority_section, "Verbosity level:", - controls["verbosity_level"], + lambda parent: wx.Choice( + parent, + choices=[ + "Minimal (essentials only)", + "Standard (recommended)", + "Detailed (all available info)", + ], + ), ) controls["severe_weather_override"] = wx.CheckBox( panel, diff --git a/src/accessiweather/ui/dialogs/settings_tabs/general.py b/src/accessiweather/ui/dialogs/settings_tabs/general.py index 93b8f011..430c1d7f 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/general.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/general.py @@ -36,12 +36,11 @@ def create(self, page_label: str = "General"): "Weather refresh", "These settings affect general app behavior for everyday use.", ) - controls["update_interval"] = wx.SpinCtrl(panel, min=1, max=120, initial=10) - self.dialog.add_labeled_row( + controls["update_interval"] = self.dialog.add_labeled_control_row( panel, refresh_section, "Refresh weather every (minutes):", - controls["update_interval"], + lambda parent: wx.SpinCtrl(parent, min=1, max=120, initial=10), ) controls["show_nationwide"] = wx.CheckBox( @@ -92,16 +91,15 @@ def create(self, page_label: str = "General"): 10, ) - controls["taskbar_icon_text_format"] = wx.TextCtrl( - panel, - size=(320, -1), - style=wx.TE_READONLY, - ) - self.dialog.add_labeled_row( + controls["taskbar_icon_text_format"] = self.dialog.add_labeled_control_row( panel, tray_section, "Current tray text format:", - controls["taskbar_icon_text_format"], + lambda parent: wx.TextCtrl( + parent, + size=(320, -1), + style=wx.TE_READONLY, + ), expand_control=True, ) controls["taskbar_icon_text_format_dialog"] = wx.Button( diff --git a/src/accessiweather/ui/dialogs/settings_tabs/notifications.py b/src/accessiweather/ui/dialogs/settings_tabs/notifications.py index 215a6414..7eee00e9 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/notifications.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/notifications.py @@ -60,20 +60,19 @@ def create(self, page_label: str = "Alerts"): "Coverage and severity", "Smaller alert areas are quieter. Broader areas catch more alerts but can be noisier.", ) - controls["alert_radius_type"] = wx.Choice( - panel, - choices=[ - "County (recommended)", - "Point (exact coordinate, may miss alerts)", - "Zone (slightly broader than county)", - "State (broadest and noisiest)", - ], - ) - self.dialog.add_labeled_row( + controls["alert_radius_type"] = self.dialog.add_labeled_control_row( panel, coverage_section, "Alert area:", - controls["alert_radius_type"], + lambda parent: wx.Choice( + parent, + choices=[ + "County (recommended)", + "Point (exact coordinate, may miss alerts)", + "Zone (slightly broader than county)", + "State (broadest and noisiest)", + ], + ), ) controls["notify_extreme"] = wx.CheckBox( panel, @@ -161,12 +160,11 @@ def create(self, page_label: str = "Alerts"): "Rate limiting", "Use the advanced timing dialog if you need cooldown and freshness controls beyond the hourly limit.", ) - controls["max_notifications"] = wx.SpinCtrl(panel, min=1, max=100, initial=10) - self.dialog.add_labeled_row( + controls["max_notifications"] = self.dialog.add_labeled_control_row( panel, rate_section, "Maximum notifications per hour:", - controls["max_notifications"], + lambda parent: wx.SpinCtrl(parent, min=1, max=100, initial=10), ) advanced_btn = wx.Button(panel, label="Advanced timing...") advanced_btn.SetName("Advanced alert timing settings") diff --git a/src/accessiweather/ui/dialogs/settings_tabs/updates.py b/src/accessiweather/ui/dialogs/settings_tabs/updates.py index 20f0f00c..c7a2055c 100644 --- a/src/accessiweather/ui/dialogs/settings_tabs/updates.py +++ b/src/accessiweather/ui/dialogs/settings_tabs/updates.py @@ -43,25 +43,23 @@ def create(self, page_label: str = "Updates"): wx.LEFT | wx.RIGHT | wx.BOTTOM | wx.EXPAND, 10, ) - controls["update_channel"] = wx.Choice( - panel, - choices=[ - "Stable (production releases only)", - "Development (latest features, may be unstable)", - ], - ) - self.dialog.add_labeled_row( + controls["update_channel"] = self.dialog.add_labeled_control_row( panel, auto_section, "Release channel:", - controls["update_channel"], + lambda parent: wx.Choice( + parent, + choices=[ + "Stable (production releases only)", + "Development (latest features, may be unstable)", + ], + ), ) - controls["update_check_interval"] = wx.SpinCtrl(panel, min=1, max=168, initial=24) - self.dialog.add_labeled_row( + controls["update_check_interval"] = self.dialog.add_labeled_control_row( panel, auto_section, "Check every (hours):", - controls["update_check_interval"], + lambda parent: wx.SpinCtrl(parent, min=1, max=168, initial=24), ) manual_section = self.dialog.create_section( diff --git a/tests/test_dialog_accessibility.py b/tests/test_dialog_accessibility.py index 4c8e2429..34d2bdf1 100644 --- a/tests/test_dialog_accessibility.py +++ b/tests/test_dialog_accessibility.py @@ -92,7 +92,14 @@ def AddStretchSpacer(self): monkeypatch.setattr(settings_module.wx, "StaticText", FakeWindow, raising=False) monkeypatch.setattr(settings_module.wx, "BoxSizer", FakeSizer, raising=False) monkeypatch.setattr(settings_module.wx, "ScrolledWindow", FakeScrolledWindow, raising=False) - monkeypatch.setattr(settings_module.wx, "StaticBoxSizer", FakeSizer, raising=False) + monkeypatch.setattr( + settings_module.wx, + "StaticBoxSizer", + lambda *args, **kwargs: (_ for _ in ()).throw( + AssertionError("StaticBoxSizer should not be used in the event sounds dialog") + ), + raising=False, + ) monkeypatch.setattr(settings_module.wx, "CheckBox", FakeCheckBox, raising=False) monkeypatch.setattr(settings_module.wx, "Button", FakeButton, raising=False) for name in [ diff --git a/tests/test_settings_dialog_ux_metadata.py b/tests/test_settings_dialog_ux_metadata.py index 2a861e66..3af8f2ea 100644 --- a/tests/test_settings_dialog_ux_metadata.py +++ b/tests/test_settings_dialog_ux_metadata.py @@ -1,5 +1,6 @@ from __future__ import annotations +from types import SimpleNamespace from unittest.mock import MagicMock import accessiweather.ui.dialogs.settings_dialog as settings_module @@ -97,3 +98,129 @@ def test_create_section_uses_heading_and_plain_sizer_for_accessibility(monkeypat dialog._wrap_static_text.assert_called_once_with(fake_heading) dialog.add_help_text.assert_not_called() assert parent_sizer.Add.call_count == 2 + + +def test_add_labeled_control_row_creates_label_before_control(monkeypatch): + creation_order: list[str] = [] + fake_row = MagicMock() + fake_label = MagicMock() + fake_control = MagicMock() + + monkeypatch.setattr( + settings_module.wx, + "BoxSizer", + MagicMock(return_value=fake_row), + raising=False, + ) + monkeypatch.setattr(settings_module.wx, "HORIZONTAL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "ALIGN_CENTER_VERTICAL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "RIGHT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "LEFT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "BOTTOM", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "EXPAND", 0, raising=False) + + def fake_static_text(parent, label): + creation_order.append(f"label:{label}") + return fake_label + + monkeypatch.setattr(settings_module.wx, "StaticText", fake_static_text, raising=False) + + dialog = SettingsDialogSimple.__new__(SettingsDialogSimple) + parent_sizer = MagicMock() + parent = MagicMock() + + control = dialog.add_labeled_control_row( + parent, + parent_sizer, + "Weather source:", + lambda _parent: creation_order.append("control") or fake_control, + ) + + assert control is fake_control + assert creation_order == ["label:Weather source:", "control"] + assert fake_row.Add.call_count == 2 + parent_sizer.Add.assert_called_once() + + +def test_data_sources_tab_provider_groups_avoid_static_box_sizers(monkeypatch): + class FakeControl: + def __init__(self, *args, **kwargs): + self.args = args + self.kwargs = kwargs + self.bound = [] + self.sizer = None + + def Bind(self, *args, **kwargs): + self.bound.append((args, kwargs)) + + def SetScrollRate(self, *args): + return None + + def SetSizer(self, sizer): + self.sizer = sizer + + class FakeSizer: + def __init__(self, *args, **kwargs): + self.children = [] + + def Add(self, *args, **kwargs): + self.children.append((args, kwargs)) + + def ShowItems(self, _show): + return None + + class FakeNotebook(FakeControl): + def __init__(self): + super().__init__() + self.pages = [] + + def AddPage(self, panel, label): + self.pages.append((panel, label)) + + monkeypatch.setattr( + settings_module.wx, + "StaticBoxSizer", + MagicMock(side_effect=AssertionError("StaticBoxSizer should not be used")), + raising=False, + ) + monkeypatch.setattr(settings_module.wx, "ScrolledWindow", FakeControl, raising=False) + monkeypatch.setattr(settings_module.wx, "BoxSizer", FakeSizer, raising=False) + monkeypatch.setattr(settings_module.wx, "Choice", FakeControl, raising=False) + monkeypatch.setattr(settings_module.wx, "TextCtrl", FakeControl, raising=False) + monkeypatch.setattr(settings_module.wx, "Button", FakeControl, raising=False) + monkeypatch.setattr(settings_module.wx, "StaticText", FakeControl, raising=False) + monkeypatch.setattr(settings_module.wx, "TE_MULTILINE", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "TE_NO_VSCROLL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "TE_READONLY", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "TE_PASSWORD", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "VERTICAL", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "LEFT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "RIGHT", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "BOTTOM", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "EXPAND", 0, raising=False) + monkeypatch.setattr(settings_module.wx, "EVT_CHOICE", object(), raising=False) + monkeypatch.setattr(settings_module.wx, "EVT_BUTTON", object(), raising=False) + + dialog = SimpleNamespace( + notebook=FakeNotebook(), + _controls={}, + add_help_text=MagicMock(), + create_section=MagicMock(side_effect=lambda *args, **kwargs: FakeSizer()), + add_labeled_row=MagicMock(), + add_labeled_control_row=MagicMock( + side_effect=lambda _p, _s, _l, factory, **_k: factory(_p) + ), + _wrap_static_text=MagicMock(), + _on_data_source_changed=MagicMock(), + _on_configure_source_settings=MagicMock(), + _on_get_vc_api_key=MagicMock(), + _on_validate_vc_api_key=MagicMock(), + _on_get_pw_api_key=MagicMock(), + _on_validate_pw_api_key=MagicMock(), + ) + + tab = DataSourcesTab(dialog) + panel = tab.create() + + assert panel is dialog.notebook.pages[0][0] + assert dialog.create_section.call_count >= 3