From bd8d94d1b5b3729da63545120955f299ee6b25eb Mon Sep 17 00:00:00 2001 From: Daniel Viegas Date: Sun, 30 Nov 2025 01:08:42 +0100 Subject: [PATCH] Final code quality improvements: extract constants and add helper methods - 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 --- MyTimeline.py | 145 ++++++++++++++++++++++++++++---------------------- 1 file changed, 80 insertions(+), 65 deletions(-) diff --git a/MyTimeline.py b/MyTimeline.py index f20b59f..ced5d1b 100644 --- a/MyTimeline.py +++ b/MyTimeline.py @@ -115,6 +115,10 @@ YEAR_SELECTOR_SPACING = 5 # Spacing in year selector boxes TOOLBAR_SPACING = 5 # Spacing in toolbar TOOLBAR_ITEM_MARGIN = 10 # Margin for toolbar items 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_FAMILY = "Sans" @@ -131,6 +135,14 @@ SHADOW_OFFSET_X = 1 SHADOW_OFFSET_Y = 1 SHADOW_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_COLOR = (0.2, 0.5, 1.0, 0.75) # Brighter, more opaque blue @@ -468,6 +480,13 @@ class MyTimelineView(NavigationView): """ 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: """ Called when the page changes. @@ -482,8 +501,7 @@ class MyTimelineView(NavigationView): # If no event is selected, ensure timeline is displayed if not self.events and self.dbstate.is_open(): self.collect_events() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() 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 self.collect_events() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() 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 self.collect_events() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() def event_updated(self, handle_list: List[str]) -> None: """ @@ -518,8 +534,7 @@ class MyTimelineView(NavigationView): """ # Re-collect events when events are updated self.collect_events() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() # If the active event was updated, ensure it's still highlighted if self.active_event_handle in handle_list: @@ -547,8 +562,7 @@ class MyTimelineView(NavigationView): db.connect("person-update", self.person_updated) db.connect("event-update", self.event_updated) - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() def build_widget(self) -> Gtk.Widget: """ @@ -761,11 +775,11 @@ class MyTimelineView(NavigationView): # Main content area content_area = dialog.get_content_area() - content_area.set_spacing(10) - content_area.set_margin_start(10) - content_area.set_margin_end(10) - content_area.set_margin_top(10) - content_area.set_margin_bottom(10) + content_area.set_spacing(FILTER_PAGE_SPACING) + content_area.set_margin_start(FILTER_PAGE_MARGIN) + content_area.set_margin_end(FILTER_PAGE_MARGIN) + content_area.set_margin_top(FILTER_PAGE_MARGIN) + content_area.set_margin_bottom(FILTER_PAGE_MARGIN) # Notebook for organizing filters notebook = Gtk.Notebook() @@ -1079,6 +1093,21 @@ class MyTimelineView(NavigationView): scrolled.add(box) 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"{message}" + ) + else: + self.date_validation_label.set_text("") + def _update_year_spin_button(self, spin_key: str, year: int, handler: Callable) -> None: """ 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) # Clear validation message - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_text("") + self._set_validation_message("") def _update_filter_dialog_state(self) -> None: """ @@ -1353,10 +1381,7 @@ class MyTimelineView(NavigationView): if from_date is None or to_date is None: # Show error message for invalid dates - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_markup( - f"{_('Error: Invalid date in date range filter')}" - ) + self._set_validation_message(_('Error: Invalid date in date range filter')) logger.warning("Error parsing date range in filter dialog: invalid date from calendar") self.date_range_filter = None self.date_range_explicit = False @@ -1368,17 +1393,13 @@ class MyTimelineView(NavigationView): # Validate date range if min_sort > max_sort: # Show error message - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_markup( - f"{_('Error: From date must be before To date')}" - ) + self._set_validation_message(_('Error: From date must be before To date')) self.date_range_filter = None self.date_range_explicit = False return else: # Clear error message - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_text("") + self._set_validation_message("") # Set filter - user has selected dates in calendars 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) # Clear validation message - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_text("") + self._set_validation_message("") # Mark that date range should not be applied self.date_range_explicit = False @@ -1595,23 +1615,21 @@ class MyTimelineView(NavigationView): from_year, from_month, from_day = from_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: - from_date = Date() - from_date.set_yr_mon_day(from_year, from_month + 1, from_day) 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() if from_sort > to_sort: - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_markup( - f"{_('Warning: From date is after To date')}" - ) + self._set_validation_message(_('Warning: From date is after To date')) else: - if hasattr(self, 'date_validation_label'): - self.date_validation_label.set_text("") + self._set_validation_message("") except (ValueError, AttributeError, TypeError): pass @@ -1627,8 +1645,8 @@ class MyTimelineView(NavigationView): active_handle = self.get_active() if active_handle: self.goto_handle(active_handle) - elif self.drawing_area: - self.drawing_area.queue_draw() + else: + self._queue_draw() def goto_handle(self, handle: str) -> None: """ @@ -1647,8 +1665,7 @@ class MyTimelineView(NavigationView): self.collect_events() else: # Just refresh display to highlight the selected event - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() # Scroll to the selected event if possible self._scroll_to_event(handle) @@ -2175,8 +2192,7 @@ class MyTimelineView(NavigationView): self.events = self._apply_filters(self.all_events) self._invalidate_cache() self._calculate_timeline_height() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() def _calculate_date_range(self) -> Tuple[int, int, int]: """ @@ -2418,8 +2434,7 @@ class MyTimelineView(NavigationView): self.zoom_level = level self.update_zoom_display() self._recalculate_timeline_height() - if self.drawing_area: - self.drawing_area.queue_draw() + self._queue_draw() def on_zoom_in(self, widget: Gtk.Widget) -> None: """ @@ -2518,7 +2533,7 @@ class MyTimelineView(NavigationView): self.selected_person_handles.add(person_handle) # Color will be assigned when drawing - self.drawing_area.queue_draw() + self._queue_draw() return False 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 ) - self.drawing_area.queue_draw() + self._queue_draw() return False @@ -2584,7 +2599,7 @@ class MyTimelineView(NavigationView): self.tooltip_window.destroy() self.tooltip_window = None - self.drawing_area.queue_draw() + self._queue_draw() return False 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: 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) # Get parent window for proper display @@ -2838,18 +2853,18 @@ class MyTimelineView(NavigationView): label = Gtk.Label() label.set_markup(tooltip_text) label.set_line_wrap(True) - label.set_max_width_chars(40) - label.set_margin_start(5) - label.set_margin_end(5) - label.set_margin_top(5) - label.set_margin_bottom(5) + label.set_max_width_chars(TOOLTIP_MAX_WIDTH_CHARS) + label.set_margin_start(TOOLTIP_LABEL_MARGIN) + label.set_margin_end(TOOLTIP_LABEL_MARGIN) + label.set_margin_top(TOOLTIP_LABEL_MARGIN) + label.set_margin_bottom(TOOLTIP_LABEL_MARGIN) frame.add(label) tooltip_window.add(frame) tooltip_window.show_all() # 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 @@ -2863,8 +2878,8 @@ class MyTimelineView(NavigationView): height: Widget 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(1, 0.95, 0.96, 0.98) # Slightly darker + pattern.add_color_stop_rgb(0, *BACKGROUND_GRADIENT_START) + pattern.add_color_stop_rgb(1, *BACKGROUND_GRADIENT_END) context.set_source(pattern) context.paint() @@ -2897,16 +2912,16 @@ class MyTimelineView(NavigationView): y_end: Y coordinate of the timeline end. """ # 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.move_to(timeline_x + 2, y_start + 2) - context.line_to(timeline_x + 2, y_end + 2) + context.move_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_start + TIMELINE_SHADOW_OFFSET) + context.line_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_end + TIMELINE_SHADOW_OFFSET) context.stroke() # Draw main line with gradient 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(1, 0.5, 0.5, 0.5) + pattern.add_color_stop_rgb(0, *TIMELINE_AXIS_GRADIENT_START) + pattern.add_color_stop_rgb(1, *TIMELINE_AXIS_GRADIENT_END) context.set_source(pattern) context.set_line_width(TIMELINE_LINE_WIDTH) context.move_to(timeline_x, y_start)