-
Notifications
You must be signed in to change notification settings - Fork 11.2k
feat: Overhaul UI with Theme Toggle, Tooltips, and Layout Adjustments #1588
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Added a persistent theme toggle (Light/Dark) with icons, saving the user's preference. - Implemented a robust ToolTip manager to fix flickering and improve stability. - Added info buttons with detailed tooltips for all major settings to improve user experience. - Refactored UI layout for better organization and component placement.
- Increased main window width for better component spacing. - Refined the placement of help icons by removing relative sizing to ensure consistent positioning.
Reviewer's GuideMajor UI overhaul in modules/ui.py introducing a centralized tooltip manager, a persistent light/dark theme toggle with icon button, expanded window width, and contextual info “?” buttons with tooltips for all major switches, along with layout adjustments for better spacing and stability. Sequence diagram for centralized tooltip show and hide behaviorsequenceDiagram
actor User as "User"
participant InfoButton as "'?' info button"
participant ToolTip as "ToolTip controller"
participant ToolTipManager as "ToolTipManager singleton"
participant TooltipWindow as "Tooltip CTkToplevel"
User->>InfoButton: "Move mouse over button (<Enter>)"
InfoButton->>ToolTip: "<Enter> event callback"
ToolTip->>ToolTipManager: "schedule_show(widget, text)"
ToolTipManager->>ToolTipManager: "cancel_timers()"
ToolTipManager->>ToolTipManager: "start 300ms delay via widget.after()"
Note over ToolTipManager,TooltipWindow: "After 300ms, if still hovered, tooltip is shown"
ToolTipManager->>ToolTipManager: "_show(widget, text)"
ToolTipManager->>TooltipWindow: "_get_tip_window(parent) (create or reuse)"
TooltipWindow-->>ToolTipManager: "returns existing or new window"
ToolTipManager->>TooltipWindow: "configure label text and geometry"
ToolTipManager->>TooltipWindow: "deiconify() (show tooltip)"
ToolTipManager->>ToolTipManager: "start 3000ms hide timer via after()"
User->>InfoButton: "Move mouse away (<Leave>) or click"
InfoButton->>ToolTip: "<Leave> or <ButtonPress> event callback"
ToolTip->>ToolTipManager: "hide()"
ToolTipManager->>ToolTipManager: "cancel_timers()"
ToolTipManager->>TooltipWindow: "withdraw() (hide tooltip)"
Sequence diagram for theme toggle and preference persistencesequenceDiagram
actor User as "User"
participant ThemeButton as "Theme toggle button"
participant ThemeUtils as "Theme utilities"
participant CTk as "CustomTkinter (ctk)"
participant File as "theme_preference.json"
rect rgb(230,230,250)
CTk->>ThemeUtils: "load_theme_preference() on startup"
ThemeUtils->>File: "read theme_preference.json"
File-->>ThemeUtils: "saved theme or FileNotFoundError"
ThemeUtils-->>CTk: "return saved theme or 'system'"
CTk->>CTk: "set_appearance_mode(saved_theme)"
end
User->>ThemeButton: "Click to toggle theme"
ThemeButton->>ThemeUtils: "toggle_theme()"
ThemeUtils->>CTk: "get_appearance_mode()"
CTk-->>ThemeUtils: "current mode ('Dark' or 'Light')"
ThemeUtils->>CTk: "set_appearance_mode('light' or 'dark')"
ThemeUtils->>ThemeUtils: "save_theme_preference()"
ThemeUtils->>File: "write current theme to theme_preference.json"
ThemeUtils->>ThemeButton: "update_theme_button(theme_button)"
ThemeButton-->>User: "Updated icon and color indicating new theme"
Updated class diagram for tooltip management and theme utilitiesclassDiagram
class ToolTipManager {
- _tip_window
- _label
- _show_timer
- _hide_timer
+ _get_tip_window()
+ schedule_show(widget, text)
+ hide()
+ cancel_timers()
}
class ToolTip {
- widget
- text
+ ToolTip(widget, text)
+ on_enter(event)
+ on_leave(event)
}
class ThemeUtilities {
+ get_theme_icon_char(mode)
+ get_theme_icon_color(mode)
+ update_theme_button(theme_button)
+ toggle_theme()
+ save_theme_preference()
+ load_theme_preference()
}
class UIGlobals {
- ROOT_HEIGHT = 750
- ROOT_WIDTH = 800
- ROOT
- theme_button
}
ToolTip ..> ToolTipManager : uses for tooltip show/hide
ToolTipManager ..> UIGlobals : reads ROOT for positioning
ThemeUtilities ..> UIGlobals : updates theme_button
ThemeUtilities ..> UIGlobals : uses global theme state
Flow diagram for tooltip lifecycle and window interactionsflowchart TD
A["User hovers over '?' info button"] --> B["ToolTip.on_enter() is called"]
B --> C["ToolTipManager.schedule_show(widget, text)"]
C --> D["ToolTipManager.cancel_timers()"]
D --> E["Start 300ms show timer via widget.after()"]
E --> F{"Is cursor still over widget when timer fires?"}
F -- "No" --> G["Do nothing (tooltip remains hidden)"]
F -- "Yes" --> H["ToolTipManager._show(widget, text)"]
H --> I["Get or create shared tooltip window (CTkToplevel)"]
I --> J["Update label text and compute position within main window"]
J --> K["Set tooltip geometry and deiconify() to show"]
K --> L["Start 3000ms auto-hide timer"]
L --> M["Auto-hide timer fires"]
M --> N["ToolTipManager.hide(): cancel timers and withdraw()"]
K --> O["Window resize or focus events"]
O --> P["close_all_tooltips() -> ToolTipManager.hide()"]
K --> Q["User moves mouse away or clicks button"]
Q --> R["ToolTip.on_leave() calls ToolTipManager.hide()"]
R --> N
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Consider storing
theme_preference.jsonviaresolve_relative_pathor a similar helper instead of the current working directory, so the theme toggle remains robust when the app is launched from different locations. - The tooltip positioning logic in
ToolTipManager._showhas multiple sequential bounds checks with overlapping conditions; extracting this into a small helper function with clearer clamping logic would make it easier to reason about and maintain. - Instead of relying on the global
theme_buttoninsidetoggle_theme, consider passing the button instance or using a callback pattern to reduce global state coupling and make the UI setup easier to reuse or test.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider storing `theme_preference.json` via `resolve_relative_path` or a similar helper instead of the current working directory, so the theme toggle remains robust when the app is launched from different locations.
- The tooltip positioning logic in `ToolTipManager._show` has multiple sequential bounds checks with overlapping conditions; extracting this into a small helper function with clearer clamping logic would make it easier to reason about and maintain.
- Instead of relying on the global `theme_button` inside `toggle_theme`, consider passing the button instance or using a callback pattern to reduce global state coupling and make the UI setup easier to reuse or test.
## Individual Comments
### Comment 1
<location> `modules/ui.py:51-54` </location>
<code_context>
+ cls._tip_window = tw = ctk.CTkToplevel(parent)
+ tw.wm_overrideredirect(1)
+ tw.attributes('-topmost', True)
+ try:
+ # This makes the window transparent to mouse events
+ tw.attributes('-disabled', True)
+ except:
+ pass
+ cls._label = ctk.CTkLabel(
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Bare except in tooltip window setup may hide unexpected errors.
This bare `except:` will hide all exceptions, including unrelated programming errors in this block. Please catch the specific Tk-related exception (e.g., `tk.TclError`) instead, so unsupported `-disabled` attributes are handled without masking other issues.
Suggested implementation:
```python
try:
# This makes the window transparent to mouse events
tw.attributes('-disabled', True)
except tk.TclError:
# Some platforms/versions may not support the '-disabled' attribute
pass
```
To use tk.TclError, ensure that tkinter is imported in this module, typically near the top of modules/ui.py:
- If not already present, add:
import tkinter as tk
If the project uses a different alias or import style for tkinter (e.g., from tkinter import TclError), adjust the exception clause accordingly (e.g., except TclError:).
</issue_to_address>
### Comment 2
<location> `modules/ui.py:54` </location>
<code_context>
@classmethod
def _get_tip_window(cls, parent):
if cls._tip_window is None or not cls._tip_window.winfo_exists():
cls._tip_window = tw = ctk.CTkToplevel(parent)
tw.wm_overrideredirect(1)
tw.attributes('-topmost', True)
try:
# This makes the window transparent to mouse events
tw.attributes('-disabled', True)
except:
pass
cls._label = ctk.CTkLabel(
tw,
text="",
fg_color="#2B2B2B",
text_color="white",
font=("Arial", 10, "normal"),
corner_radius=8,
padx=12,
pady=8,
wraplength=250
)
cls._label.pack()
cls._tip_window.withdraw()
return cls._tip_window
</code_context>
<issue_to_address>
**suggestion (code-quality):** Use `except Exception:` rather than bare `except:` ([`do-not-use-bare-except`](https://docs.sourcery.ai/Reference/Default-Rules/suggestions/do-not-use-bare-except/))
```suggestion
except Exception:
```
</issue_to_address>
### Comment 3
<location> `modules/ui.py:107-114` </location>
<code_context>
@classmethod
def _show(cls, widget, text):
cls._show_timer = None
if not text:
return
tw = cls._get_tip_window(widget)
cls._label.configure(text=text)
tw.update_idletasks()
tooltip_width = tw.winfo_reqwidth()
tooltip_height = tw.winfo_reqheight()
button_x = widget.winfo_rootx()
button_y = widget.winfo_rooty()
button_width = widget.winfo_width()
button_height = widget.winfo_height()
main_window = ROOT
main_x = main_window.winfo_rootx()
main_y = main_window.winfo_rooty()
main_width = main_window.winfo_width()
main_height = main_window.winfo_height()
main_right = main_x + main_width
main_bottom = main_y + main_height
x = button_x + button_width + 10
y = button_y + 2
if x + tooltip_width > main_right - 10:
x = button_x - tooltip_width - 10
if x < main_x + 5:
x = main_x + 5
if x + tooltip_width > main_right - 5:
x = main_right - tooltip_width - 5
if y + tooltip_height > main_bottom - 5:
y = button_y - tooltip_height - 10
if y < main_y + 5:
y = main_y + 5
if x + tooltip_width > main_right:
x = main_right - tooltip_width - 5
if x < main_x:
x = main_x + 5
tw.geometry(f"+{x}+{y}")
tw.deiconify()
cls._hide_timer = tw.after(3000, cls.hide)
</code_context>
<issue_to_address>
**issue (code-quality):** Replace comparison with min/max call [×2] ([`min-max-identity`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/min-max-identity/))
</issue_to_address>
### Comment 4
<location> `modules/ui.py:180-183` </location>
<code_context>
def get_theme_icon_char(mode):
"""Get professional icon characters for theme toggle"""
if mode == "Dark":
return "☀" # Sun icon - professional Unicode
else:
return "☾" # Moon icon - professional Unicode
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return "☀" if mode == "Dark" else "☾"
```
</issue_to_address>
### Comment 5
<location> `modules/ui.py:188-191` </location>
<code_context>
def get_theme_icon_color(mode):
"""Get appropriate colors for theme icons"""
if mode == "Dark":
return "#FFD700" # Golden yellow for sun
else:
return "#4682B4" # Steel blue for moon
</code_context>
<issue_to_address>
**suggestion (code-quality):** Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return "#FFD700" if mode == "Dark" else "#4682B4"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if mode == "Dark": | ||
| return "☀" # Sun icon - professional Unicode | ||
| else: | ||
| return "☾" # Moon icon - professional Unicode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Replace if statement with if expression (assign-if-exp)
| if mode == "Dark": | |
| return "☀" # Sun icon - professional Unicode | |
| else: | |
| return "☾" # Moon icon - professional Unicode | |
| return "☀" if mode == "Dark" else "☾" |
| if mode == "Dark": | ||
| return "#FFD700" # Golden yellow for sun | ||
| else: | ||
| return "#4682B4" # Steel blue for moon |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (code-quality): Replace if statement with if expression (assign-if-exp)
| if mode == "Dark": | |
| return "#FFD700" # Golden yellow for sun | |
| else: | |
| return "#4682B4" # Steel blue for moon | |
| return "#FFD700" if mode == "Dark" else "#4682B4" |
This pull request introduces a major user interface overhaul with the following enhancements:
Theme Management: Added a persistent theme toggle (Light/Dark) with professional icons, saving the user's preference across sessions.
Improved Tooltips: Implemented a robust, centralized
ToolTipManagerto fix flickering bugs and improve the stability andpositioning of informational tooltips.
Info Buttons: Added clear '?' info buttons with detailed tooltips for all major settings, significantly improving user
experience and clarity.
UI Layout Refinements:
- Refactored the UI layout for better organization and component placement.
- Increased the main window width to
800pxfor better spacing.- Adjusted the placement of help icons to ensure consistent positioning.
Summary by Sourcery
Overhaul the main UI to add persistent theme toggling, centralized tooltip handling, and clearer setting explanations while widening the main window layout.
New Features:
Bug Fixes:
Enhancements: