dv/mygramps
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Final code quality improvements: extract constants and add helper methods

Browse Source 
- Extract tooltip-related constants
  Added TOOLTIP_OFFSET, TOOLTIP_LABEL_MARGIN, TOOLTIP_MAX_WIDTH_CHARS, TOOLTIP_BORDER_WIDTH
  Replaced magic numbers in show_tooltip() and tooltip window creation

- Extract drawing-related constants
  Added TIMELINE_SHADOW_OFFSET, TIMELINE_SHADOW_OPACITY
  Extracted background and timeline axis gradient colors to constants
  Grouped color constants in dedicated Drawing Color Constants section

- Use existing constants in filter dialog
  Replaced hardcoded spacing/margin values with FILTER_PAGE_SPACING and FILTER_PAGE_MARGIN

- Add _queue_draw() helper method
  Created helper to safely queue drawing area redraw
  Replaced 12+ occurrences of 'if self.drawing_area: self.drawing_area.queue_draw()' pattern

- Add _set_validation_message() helper method
  Created helper for setting date validation messages
  Replaced 7+ occurrences of hasattr checks and manual label updates
  Provides consistent validation message handling

All magic numbers extracted, code patterns consolidated, maintainability improved
This commit is contained in:
Daniel Viegas 2025-11-30 01:08:42 +01:00
parent 6dc5f16a94
commit bd8d94d1b5
1 changed files with 80 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

145
MyTimeline.py
View File

@ -115,6 +115,10 @@ YEAR_SELECTOR_SPACING = 5 # Spacing in year selector boxes
TOOLBAR_SPACING = 5 # Spacing in toolbar TOOLBAR_SPACING = 5 # Spacing in toolbar
TOOLBAR_ITEM_MARGIN = 10 # Margin for toolbar items TOOLBAR_ITEM_MARGIN = 10 # Margin for toolbar items
TOOLTIP_SEPARATOR_LENGTH = 30 # Length of tooltip separator line TOOLTIP_SEPARATOR_LENGTH = 30 # Length of tooltip separator line
TOOLTIP_OFFSET = 15 # Offset for tooltip position to avoid cursor
TOOLTIP_LABEL_MARGIN = 5 # Margin for tooltip label
TOOLTIP_MAX_WIDTH_CHARS = 40 # Maximum width in characters for tooltip label
TOOLTIP_BORDER_WIDTH = 8 # Border width for tooltip window
# Font Constants # Font Constants
FONT_FAMILY = "Sans" FONT_FAMILY = "Sans"
@ -131,6 +135,14 @@ SHADOW_OFFSET_X = 1
SHADOW_OFFSET_Y = 1 SHADOW_OFFSET_Y = 1
SHADOW_OPACITY = 0.3 SHADOW_OPACITY = 0.3
BORDER_OPACITY = 0.3 BORDER_OPACITY = 0.3
TIMELINE_SHADOW_OFFSET = 2 # Shadow offset for timeline axis
# Drawing Color Constants
BACKGROUND_GRADIENT_START = (0.98, 0.98, 0.99) # Very light gray-blue
BACKGROUND_GRADIENT_END = (0.95, 0.96, 0.98) # Slightly darker
TIMELINE_AXIS_GRADIENT_START = (0.3, 0.3, 0.3) # Dark gray
TIMELINE_AXIS_GRADIENT_END = (0.5, 0.5, 0.5) # Medium gray
TIMELINE_SHADOW_OPACITY = 0.2 # Shadow opacity for timeline axis
# Connection Line Constants # Connection Line Constants
CONNECTION_LINE_COLOR = (0.2, 0.5, 1.0, 0.75) # Brighter, more opaque blue CONNECTION_LINE_COLOR = (0.2, 0.5, 1.0, 0.75) # Brighter, more opaque blue
@ -468,6 +480,13 @@ class MyTimelineView(NavigationView):
""" """
return "Event" return "Event"
def _queue_draw(self) -> None:
"""
Safely queue a redraw of the drawing area if it exists.
"""
if self.drawing_area:
self.drawing_area.queue_draw()
def change_page(self) -> None: def change_page(self) -> None:
""" """
Called when the page changes. Called when the page changes.
@ -482,8 +501,7 @@ class MyTimelineView(NavigationView):
# If no event is selected, ensure timeline is displayed # If no event is selected, ensure timeline is displayed
if not self.events and self.dbstate.is_open(): if not self.events and self.dbstate.is_open():
self.collect_events() self.collect_events()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def family_updated(self, handle_list: List[str]) -> None: def family_updated(self, handle_list: List[str]) -> None:
""" """
@ -494,8 +512,7 @@ class MyTimelineView(NavigationView):
""" """
# Refresh timeline since family updates may affect event display # Refresh timeline since family updates may affect event display
self.collect_events() self.collect_events()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def person_updated(self, handle_list: List[str]) -> None: def person_updated(self, handle_list: List[str]) -> None:
""" """
@ -506,8 +523,7 @@ class MyTimelineView(NavigationView):
""" """
# Refresh timeline since person updates may affect event display # Refresh timeline since person updates may affect event display
self.collect_events() self.collect_events()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def event_updated(self, handle_list: List[str]) -> None: def event_updated(self, handle_list: List[str]) -> None:
""" """
@ -518,8 +534,7 @@ class MyTimelineView(NavigationView):
""" """
# Re-collect events when events are updated # Re-collect events when events are updated
self.collect_events() self.collect_events()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
# If the active event was updated, ensure it's still highlighted # If the active event was updated, ensure it's still highlighted
if self.active_event_handle in handle_list: if self.active_event_handle in handle_list:
@ -547,8 +562,7 @@ class MyTimelineView(NavigationView):
db.connect("person-update", self.person_updated) db.connect("person-update", self.person_updated)
db.connect("event-update", self.event_updated) db.connect("event-update", self.event_updated)
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def build_widget(self) -> Gtk.Widget: def build_widget(self) -> Gtk.Widget:
""" """
@ -761,11 +775,11 @@ class MyTimelineView(NavigationView):
# Main content area # Main content area
content_area = dialog.get_content_area() content_area = dialog.get_content_area()
content_area.set_spacing(10) content_area.set_spacing(FILTER_PAGE_SPACING)
content_area.set_margin_start(10) content_area.set_margin_start(FILTER_PAGE_MARGIN)
content_area.set_margin_end(10) content_area.set_margin_end(FILTER_PAGE_MARGIN)
content_area.set_margin_top(10) content_area.set_margin_top(FILTER_PAGE_MARGIN)
content_area.set_margin_bottom(10) content_area.set_margin_bottom(FILTER_PAGE_MARGIN)
# Notebook for organizing filters # Notebook for organizing filters
notebook = Gtk.Notebook() notebook = Gtk.Notebook()
@ -1079,6 +1093,21 @@ class MyTimelineView(NavigationView):
scrolled.add(box) scrolled.add(box)
return scrolled return scrolled
def _set_validation_message(self, message: str) -> None:
"""
Set the date validation label message.
Args:
message: Message text to display (empty string to clear).
"""
if hasattr(self, 'date_validation_label'):
if message:
self.date_validation_label.set_markup(
f"<span color='red'>{message}</span>"
)
else:
self.date_validation_label.set_text("")
def _update_year_spin_button(self, spin_key: str, year: int, handler: Callable) -> None: def _update_year_spin_button(self, spin_key: str, year: int, handler: Callable) -> None:
""" """
Update a year spin button value, blocking signals during update. Update a year spin button value, blocking signals during update.
@ -1271,8 +1300,7 @@ class MyTimelineView(NavigationView):
self._update_year_spin_button('date_to_year_spin', now.year, self._on_to_year_changed) self._update_year_spin_button('date_to_year_spin', now.year, self._on_to_year_changed)
# Clear validation message # Clear validation message
if hasattr(self, 'date_validation_label'): self._set_validation_message("")
self.date_validation_label.set_text("")
def _update_filter_dialog_state(self) -> None: def _update_filter_dialog_state(self) -> None:
""" """
@ -1353,10 +1381,7 @@ class MyTimelineView(NavigationView):
if from_date is None or to_date is None: if from_date is None or to_date is None:
# Show error message for invalid dates # Show error message for invalid dates
if hasattr(self, 'date_validation_label'): self._set_validation_message(_('Error: Invalid date in date range filter'))
self.date_validation_label.set_markup(
f"<span color='red'>{_('Error: Invalid date in date range filter')}</span>"
)
logger.warning("Error parsing date range in filter dialog: invalid date from calendar") logger.warning("Error parsing date range in filter dialog: invalid date from calendar")
self.date_range_filter = None self.date_range_filter = None
self.date_range_explicit = False self.date_range_explicit = False
@ -1368,17 +1393,13 @@ class MyTimelineView(NavigationView):
# Validate date range # Validate date range
if min_sort > max_sort: if min_sort > max_sort:
# Show error message # Show error message
if hasattr(self, 'date_validation_label'): self._set_validation_message(_('Error: From date must be before To date'))
self.date_validation_label.set_markup(
f"<span color='red'>{_('Error: From date must be before To date')}</span>"
)
self.date_range_filter = None self.date_range_filter = None
self.date_range_explicit = False self.date_range_explicit = False
return return
else: else:
# Clear error message # Clear error message
if hasattr(self, 'date_validation_label'): self._set_validation_message("")
self.date_validation_label.set_text("")
# Set filter - user has selected dates in calendars # Set filter - user has selected dates in calendars
self.date_range_filter = (min_sort, max_sort) self.date_range_filter = (min_sort, max_sort)
@ -1576,8 +1597,7 @@ class MyTimelineView(NavigationView):
to_year_spin.handler_unblock_by_func(self._on_to_year_changed) to_year_spin.handler_unblock_by_func(self._on_to_year_changed)
# Clear validation message # Clear validation message
if hasattr(self, 'date_validation_label'): self._set_validation_message("")
self.date_validation_label.set_text("")
# Mark that date range should not be applied # Mark that date range should not be applied
self.date_range_explicit = False self.date_range_explicit = False
@ -1595,23 +1615,21 @@ class MyTimelineView(NavigationView):
from_year, from_month, from_day = from_calendar.get_date() from_year, from_month, from_day = from_calendar.get_date()
to_year, to_month, to_day = to_calendar.get_date() to_year, to_month, to_day = to_calendar.get_date()
# Use helper method to create dates
from_date = self._create_date_from_calendar(from_calendar)
to_date = self._create_date_from_calendar(to_calendar)
if from_date is None or to_date is None:
return
try: try:
from_date = Date()
from_date.set_yr_mon_day(from_year, from_month + 1, from_day)
from_sort = from_date.get_sort_value() from_sort = from_date.get_sort_value()
to_date = Date()
to_date.set_yr_mon_day(to_year, to_month + 1, to_day)
to_sort = to_date.get_sort_value() to_sort = to_date.get_sort_value()
if from_sort > to_sort: if from_sort > to_sort:
if hasattr(self, 'date_validation_label'): self._set_validation_message(_('Warning: From date is after To date'))
self.date_validation_label.set_markup(
f"<span color='red'>{_('Warning: From date is after To date')}</span>"
)
else: else:
if hasattr(self, 'date_validation_label'): self._set_validation_message("")
self.date_validation_label.set_text("")
except (ValueError, AttributeError, TypeError): except (ValueError, AttributeError, TypeError):
pass pass
@ -1627,8 +1645,8 @@ class MyTimelineView(NavigationView):
active_handle = self.get_active() active_handle = self.get_active()
if active_handle: if active_handle:
self.goto_handle(active_handle) self.goto_handle(active_handle)
elif self.drawing_area: else:
self.drawing_area.queue_draw() self._queue_draw()
def goto_handle(self, handle: str) -> None: def goto_handle(self, handle: str) -> None:
""" """
@ -1647,8 +1665,7 @@ class MyTimelineView(NavigationView):
self.collect_events() self.collect_events()
else: else:
# Just refresh display to highlight the selected event # Just refresh display to highlight the selected event
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
# Scroll to the selected event if possible # Scroll to the selected event if possible
self._scroll_to_event(handle) self._scroll_to_event(handle)
@ -2175,8 +2192,7 @@ class MyTimelineView(NavigationView):
self.events = self._apply_filters(self.all_events) self.events = self._apply_filters(self.all_events)
self._invalidate_cache() self._invalidate_cache()
self._calculate_timeline_height() self._calculate_timeline_height()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def _calculate_date_range(self) -> Tuple[int, int, int]: def _calculate_date_range(self) -> Tuple[int, int, int]:
""" """
@ -2418,8 +2434,7 @@ class MyTimelineView(NavigationView):
self.zoom_level = level self.zoom_level = level
self.update_zoom_display() self.update_zoom_display()
self._recalculate_timeline_height() self._recalculate_timeline_height()
if self.drawing_area: self._queue_draw()
self.drawing_area.queue_draw()
def on_zoom_in(self, widget: Gtk.Widget) -> None: def on_zoom_in(self, widget: Gtk.Widget) -> None:
""" """
@ -2518,7 +2533,7 @@ class MyTimelineView(NavigationView):
self.selected_person_handles.add(person_handle) self.selected_person_handles.add(person_handle)
# Color will be assigned when drawing # Color will be assigned when drawing
self.drawing_area.queue_draw() self._queue_draw()
return False return False
def on_motion_notify(self, widget: Gtk.Widget, event: Gdk.Event) -> bool: def on_motion_notify(self, widget: Gtk.Widget, event: Gdk.Event) -> bool:
@ -2557,7 +2572,7 @@ class MyTimelineView(NavigationView):
TOOLTIP_DELAY, self.show_tooltip, hovered_index, event.x_root, event.y_root TOOLTIP_DELAY, self.show_tooltip, hovered_index, event.x_root, event.y_root
) )
self.drawing_area.queue_draw() self._queue_draw()
return False return False
@ -2584,7 +2599,7 @@ class MyTimelineView(NavigationView):
self.tooltip_window.destroy() self.tooltip_window.destroy()
self.tooltip_window = None self.tooltip_window = None
self.drawing_area.queue_draw() self._queue_draw()
return False return False
def _get_cache_key(self, timeline_y_start: float, timeline_y_end: float) -> int: def _get_cache_key(self, timeline_y_start: float, timeline_y_end: float) -> int:
@ -2782,7 +2797,7 @@ class MyTimelineView(NavigationView):
""" """
if not hasattr(self, 'tooltip_window') or self.tooltip_window is None: if not hasattr(self, 'tooltip_window') or self.tooltip_window is None:
self.tooltip_window = Gtk.Window(type=Gtk.WindowType.POPUP) self.tooltip_window = Gtk.Window(type=Gtk.WindowType.POPUP)
self.tooltip_window.set_border_width(8) self.tooltip_window.set_border_width(TOOLTIP_BORDER_WIDTH)
self.tooltip_window.set_decorated(False) self.tooltip_window.set_decorated(False)
# Get parent window for proper display # Get parent window for proper display
@ -2838,18 +2853,18 @@ class MyTimelineView(NavigationView):
label = Gtk.Label() label = Gtk.Label()
label.set_markup(tooltip_text) label.set_markup(tooltip_text)
label.set_line_wrap(True) label.set_line_wrap(True)
label.set_max_width_chars(40) label.set_max_width_chars(TOOLTIP_MAX_WIDTH_CHARS)
label.set_margin_start(5) label.set_margin_start(TOOLTIP_LABEL_MARGIN)
label.set_margin_end(5) label.set_margin_end(TOOLTIP_LABEL_MARGIN)
label.set_margin_top(5) label.set_margin_top(TOOLTIP_LABEL_MARGIN)
label.set_margin_bottom(5) label.set_margin_bottom(TOOLTIP_LABEL_MARGIN)
frame.add(label) frame.add(label)
tooltip_window.add(frame) tooltip_window.add(frame)
tooltip_window.show_all() tooltip_window.show_all()
# Position tooltip (offset to avoid cursor) # Position tooltip (offset to avoid cursor)
tooltip_window.move(int(x_root) + 15, int(y_root) + 15) tooltip_window.move(int(x_root) + TOOLTIP_OFFSET, int(y_root) + TOOLTIP_OFFSET)
return False return False
@ -2863,8 +2878,8 @@ class MyTimelineView(NavigationView):
height: Widget height. height: Widget height.
""" """
pattern = cairo.LinearGradient(0, 0, 0, height) pattern = cairo.LinearGradient(0, 0, 0, height)
pattern.add_color_stop_rgb(0, 0.98, 0.98, 0.99) # Very light gray-blue pattern.add_color_stop_rgb(0, *BACKGROUND_GRADIENT_START)
pattern.add_color_stop_rgb(1, 0.95, 0.96, 0.98) # Slightly darker pattern.add_color_stop_rgb(1, *BACKGROUND_GRADIENT_END)
context.set_source(pattern) context.set_source(pattern)
context.paint() context.paint()
@ -2897,16 +2912,16 @@ class MyTimelineView(NavigationView):
y_end: Y coordinate of the timeline end. y_end: Y coordinate of the timeline end.
""" """
# Draw shadow # Draw shadow
context.set_source_rgba(0.0, 0.0, 0.0, 0.2) context.set_source_rgba(0.0, 0.0, 0.0, TIMELINE_SHADOW_OPACITY)
context.set_line_width(TIMELINE_LINE_WIDTH) context.set_line_width(TIMELINE_LINE_WIDTH)
context.move_to(timeline_x + 2, y_start + 2) context.move_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_start + TIMELINE_SHADOW_OFFSET)
context.line_to(timeline_x + 2, y_end + 2) context.line_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_end + TIMELINE_SHADOW_OFFSET)
context.stroke() context.stroke()
# Draw main line with gradient # Draw main line with gradient
pattern = cairo.LinearGradient(timeline_x, y_start, timeline_x, y_end) pattern = cairo.LinearGradient(timeline_x, y_start, timeline_x, y_end)
pattern.add_color_stop_rgb(0, 0.3, 0.3, 0.3) pattern.add_color_stop_rgb(0, *TIMELINE_AXIS_GRADIENT_START)
pattern.add_color_stop_rgb(1, 0.5, 0.5, 0.5) pattern.add_color_stop_rgb(1, *TIMELINE_AXIS_GRADIENT_END)
context.set_source(pattern) context.set_source(pattern)
context.set_line_width(TIMELINE_LINE_WIDTH) context.set_line_width(TIMELINE_LINE_WIDTH)
context.move_to(timeline_x, y_start) context.move_to(timeline_x, y_start)