diff --git a/CODE_ANALYSIS_V2.md b/CODE_ANALYSIS_V2.md new file mode 100644 index 0000000..9fe4f9d --- /dev/null +++ b/CODE_ANALYSIS_V2.md @@ -0,0 +1,394 @@ +# MyTimeline Plugin - Additional Code Analysis & Improvements + +## Executive Summary + +After removing expand/collapse functionality and implementing previous enhancements, the codebase is in good shape. However, there are still several opportunities for improvement in code quality, maintainability, and robustness. + +**Current State:** +- Total Lines: 1,269 +- Methods: 28 +- Average Method Length: 35.6 lines +- Some methods exceed 100 lines (complexity concern) + +--- + +## 1. Tuple Indexing Issues (MEDIUM PRIORITY) + +### 1.1 Numeric Index Access +**Location:** Throughout codebase, especially `_calculate_adjusted_positions()` (line 573) + +**Problem:** Using numeric indices like `prev_data[5]` is error-prone and hard to maintain. + +**Current Code:** +```python +prev_y_pos = prev_data[5] # Get y_pos from previous event (index 5 now) +``` + +**Impact:** +- Easy to introduce bugs when tuple structure changes +- Code is hard to read and understand +- No type safety + +**Recommendation:** Use named tuples or dataclasses: +```python +from dataclasses import dataclass + +@dataclass +class TimelineEvent: + date_sort: int + date_obj: Date + event: Event + person: Person | None + event_type: EventType + y_pos: float + +# Then use: +prev_y_pos = prev_data.y_pos # Much clearer! +``` + +**Benefits:** +- Type safety +- Better IDE support +- Self-documenting code +- Easier refactoring + +--- + +## 2. Long Methods (MEDIUM PRIORITY) + +### 2.1 `collect_person_events` - 130 lines +**Location:** Lines 457-586 + +**Problem:** This nested function is very long and handles multiple responsibilities. + +**Recommendation:** Break into smaller functions: +```python +def _collect_person_event_refs(self, person): + """Collect event references from a person.""" + # Extract event reference collection logic + +def _process_event_ref(self, event_ref, person_obj): + """Process a single event reference.""" + # Extract event processing logic +``` + +### 2.2 `show_tooltip` - 100 lines +**Location:** Lines 804-903 + +**Problem:** Handles both person events and single events with complex formatting. + +**Recommendation:** Split into helper methods: +```python +def _format_person_tooltip(self, person, person_events): + """Format tooltip for person with multiple events.""" + +def _format_single_event_tooltip(self, event, event_type): + """Format tooltip for single event.""" +``` + +### 2.3 `draw_event_marker` - 94 lines +**Location:** Lines 994-1087 + +**Problem:** Handles multiple shape types and styling logic. + +**Recommendation:** Extract shape-specific logic: +```python +def _draw_marker_shadow(self, context, x, y, size): + """Draw shadow for marker.""" + +def _draw_marker_gradient(self, context, x, y, size, color): + """Draw gradient fill for marker.""" +``` + +### 2.4 `on_draw` - 90 lines +**Location:** Lines 904-993 + +**Problem:** Handles multiple drawing responsibilities. + +**Recommendation:** Break into helper methods: +```python +def _draw_background(self, context, width, height): + """Draw background gradient.""" + +def _draw_timeline_axis(self, context, timeline_x, y_start, y_end): + """Draw timeline axis with shadow.""" + +def _draw_events(self, context, events_with_y_pos, timeline_x): + """Draw all events.""" +``` + +--- + +## 3. Cache Management (LOW PRIORITY) + +### 3.1 Cache Invalidation Logic +**Location:** `_get_adjusted_events()` (line 726) + +**Problem:** Cache invalidation checks context object identity, which may not be reliable. + +**Current Code:** +```python +if (self._adjusted_events_cache is not None and + self._cache_context == context and + self._cache_timeline_y_start == timeline_y_start and + self._cache_timeline_y_end == timeline_y_end): +``` + +**Recommendation:** Use a hash-based cache key: +```python +def _get_cache_key(self, timeline_y_start, timeline_y_end): + """Generate cache key for adjusted events.""" + return hash((len(self.events), timeline_y_start, timeline_y_end)) +``` + +**Benefits:** +- More reliable cache invalidation +- Don't need to store context object +- Better performance + +--- + +## 4. Code Organization (LOW PRIORITY) + +### 4.1 Method Grouping +**Recommendation:** Group related methods together: +- Drawing methods: `on_draw`, `draw_event_marker`, `draw_event_label`, `draw_year_markers`, `draw_person_connections` +- Event handling: `collect_events`, `collect_person_events`, `_get_adjusted_events` +- Interaction: `on_button_press`, `on_motion_notify`, `on_leave_notify`, `find_event_at_position` +- UI: `build_widget`, `build_tree`, `goto_handle` + +### 4.2 Constants Organization +**Status:** Already well-organized, but could add more: +- Add constants for colors used in drawing +- Add constants for font sizes +- Add constants for padding values + +--- + +## 5. Error Handling Improvements (LOW PRIORITY) + +### 5.1 More Specific Exceptions +**Current:** Most handlers use generic `Exception` + +**Recommendation:** Use more specific exceptions where possible: +```python +except (AttributeError, KeyError) as e: + # Handle specific database access errors +except ValueError as e: + # Handle date parsing errors +``` + +### 5.2 Logging +**Recommendation:** Add logging for debugging: +```python +import logging +logger = logging.getLogger(__name__) + +try: + # code +except Exception as e: + logger.warning(f"Error processing event: {e}", exc_info=True) +``` + +--- + +## 6. Performance Optimizations (LOW PRIORITY) + +### 6.1 Repeated Calculations +**Location:** `on_draw()`, `find_event_at_position()` + +**Problem:** Date range calculation is repeated. + +**Recommendation:** Cache date range: +```python +@property +def date_range(self): + """Calculate and cache date range.""" + if not hasattr(self, '_cached_date_range') or self._cached_date_range is None: + if self.events: + min_date = min(event[0] for event in self.events) + max_date = max(event[0] for event in self.events) + self._cached_date_range = max_date - min_date if max_date != min_date else 1 + else: + self._cached_date_range = 1 + return self._cached_date_range +``` + +### 6.2 Tooltip Window Reuse +**Location:** `show_tooltip()` (line 804) + +**Problem:** Creates new tooltip window each time. + +**Recommendation:** Reuse tooltip window: +```python +if not hasattr(self, '_tooltip_window') or self._tooltip_window is None: + self._tooltip_window = Gtk.Window(type=Gtk.WindowType.POPUP) + # ... setup +# Reuse existing window +``` + +--- + +## 7. Type Safety (LOW PRIORITY) + +### 7.1 Type Hints +**Recommendation:** Add type hints for better IDE support: +```python +from typing import Optional, List, Tuple + +def collect_events(self) -> None: +def find_event_at_position(self, x: float, y: float) -> Optional[int]: +def draw_event_marker(self, context: cairo.Context, x: float, y: float, + event_type: EventType, is_hovered: bool = False, + is_selected: bool = False) -> None: +``` + +**Benefits:** +- Better IDE autocomplete +- Catch type errors early +- Self-documenting code + +--- + +## 8. Code Duplication (LOW PRIORITY) + +### 8.1 Date Range Calculation +**Location:** `on_draw()`, `_get_adjusted_events()`, `find_event_at_position()` + +**Problem:** Date range calculation is duplicated. + +**Recommendation:** Extract to method: +```python +def _calculate_date_range(self): + """Calculate date range from events.""" + if not self.events: + return 1 + min_date = min(event[0] for event in self.events) + max_date = max(event[0] for event in self.events) + return max_date - min_date if max_date != min_date else 1 +``` + +### 8.2 Y Position Calculation +**Location:** Multiple places + +**Problem:** Y position calculation formula is repeated. + +**Recommendation:** Extract to method: +```python +def _calculate_y_position(self, date_sort, min_date, date_range, + timeline_y_start, timeline_y_end): + """Calculate Y position for an event based on date.""" + return timeline_y_start + ( + (date_sort - min_date) / date_range + ) * (timeline_y_end - timeline_y_start) +``` + +--- + +## 9. Magic Numbers (LOW PRIORITY) + +### 9.1 Remaining Magic Numbers +**Locations:** +- Line 567: `text_height + LABEL_PADDING` (already using constant, good!) +- Line 800: `500` (tooltip width?) +- Line 1125: `padding = 8` (could be constant) +- Line 1133: `radius = 5` (could be constant) + +**Recommendation:** Add constants: +```python +TOOLTIP_MAX_WIDTH = 500 +LABEL_BACKGROUND_PADDING = 8 +LABEL_BACKGROUND_RADIUS = 5 +``` + +--- + +## 10. Documentation (LOW PRIORITY) + +### 10.1 Docstring Improvements +**Recommendation:** Add parameter and return value documentation: +```python +def draw_event_marker(self, context, x, y, event_type, is_hovered=False, is_selected=False): + """ + Draw a marker for an event with modern styling. + + Args: + context: Cairo drawing context + x: X coordinate for marker center + y: Y coordinate for marker center + event_type: Type of event (determines color and shape) + is_hovered: Whether marker is currently hovered + is_selected: Whether marker belongs to selected person + + Returns: + None + """ +``` + +--- + +## Priority Summary + +### Medium Priority (Consider Soon) +1. ✅ Replace tuple indexing with named tuples/dataclasses +2. ✅ Break down long methods (collect_person_events, show_tooltip, draw_event_marker, on_draw) +3. ✅ Extract repeated calculations (date range, Y position) + +### Low Priority (Nice to Have) +1. Improve cache management +2. Add type hints +3. Add more specific exception handling +4. Add logging +5. Extract remaining magic numbers +6. Improve docstrings +7. Reuse tooltip window + +--- + +## Estimated Impact + +**Code Quality:** +- Better maintainability with named tuples +- Easier to understand with shorter methods +- Better type safety with type hints + +**Performance:** +- Slight improvement from caching date range +- Better cache management + +**Developer Experience:** +- Better IDE support +- Easier debugging with logging +- Self-documenting code + +--- + +## Implementation Order + +1. **Phase 1 (Quick Wins):** + - Extract date range calculation + - Extract Y position calculation + - Add remaining constants + +2. **Phase 2 (Refactoring):** + - Break down long methods + - Replace tuple indexing with named tuples + +3. **Phase 3 (Enhancements):** + - Add type hints + - Improve error handling + - Add logging + - Improve docstrings + +--- + +## Conclusion + +The codebase is in good shape after recent improvements. The main areas for further enhancement are: +- Using named tuples/dataclasses instead of numeric indexing +- Breaking down long methods for better maintainability +- Extracting repeated calculations +- Adding type hints for better IDE support + +These improvements would make the code more maintainable, type-safe, and easier to understand. + diff --git a/MyTimeline.py b/MyTimeline.py index 7c3a811..ba81b18 100644 --- a/MyTimeline.py +++ b/MyTimeline.py @@ -28,7 +28,11 @@ MyTimeline View - A vertical timeline showing family events # # ------------------------------------------------------------------------- import cairo +import logging import math +from dataclasses import dataclass +from typing import Optional, List, Tuple, Any + from gi.repository import Gtk from gi.repository import Gdk from gi.repository import GLib @@ -80,6 +84,12 @@ CLICKABLE_AREA_HEIGHT = 30 # UI Constants YEAR_LABEL_WIDTH = 100 TOOLTIP_DELAY = 500 # milliseconds +TOOLTIP_MAX_WIDTH = 500 +LABEL_BACKGROUND_PADDING = 8 +LABEL_BACKGROUND_RADIUS = 5 + +# Logger +logger = logging.getLogger(__name__) # Event type categories and color mapping EVENT_COLORS = { @@ -216,6 +226,22 @@ EVENT_SHAPES = { } +# ------------------------------------------------------------------------- +# +# Data Classes +# +# ------------------------------------------------------------------------- +@dataclass +class TimelineEvent: + """Represents an event in the timeline with all necessary data.""" + date_sort: int + date_obj: Date + event: 'Any' # gramps.gen.lib.Event + person: Optional['Any'] # gramps.gen.lib.Person + event_type: EventType + y_pos: float = 0.0 + + # ------------------------------------------------------------------------- # # MyTimelineView @@ -238,7 +264,7 @@ class MyTimelineView(NavigationView): # Current family handle self.active_family_handle = None - self.events = [] # List of (date_sort, date_obj, event, person, event_type, y_pos) + self.events: List[TimelineEvent] = [] # List of TimelineEvent objects # UI components self.scrolledwindow = None @@ -258,10 +284,11 @@ class MyTimelineView(NavigationView): self.mouse_y = 0 # Performance cache - self._adjusted_events_cache = None - self._cache_context = None - self._cache_timeline_y_start = None - self._cache_timeline_y_end = None + self._adjusted_events_cache: Optional[List[TimelineEvent]] = None + self._cache_key: Optional[int] = None + self._cached_date_range: Optional[int] = None + self._cached_min_date: Optional[int] = None + self._cached_max_date: Optional[int] = None # Connect to database changes self.dbstate.connect("database-changed", self.change_db) @@ -271,26 +298,45 @@ class MyTimelineView(NavigationView): self.dbstate.db.connect("person-update", self.person_updated) self.dbstate.db.connect("event-update", self.event_updated) - def navigation_type(self): - """Return the navigation type for this view.""" + def navigation_type(self) -> str: + """ + Return the navigation type for this view. + + Returns: + str: The navigation type, always "Family" for this view. + """ return "Family" - def change_page(self): - """Called when the page changes.""" + def change_page(self) -> None: + """ + Called when the page changes. + + Updates the view to show the active family's timeline. + """ NavigationView.change_page(self) active_handle = self.get_active() if active_handle: self.goto_handle(active_handle) - def family_updated(self, handle_list): - """Called when a family is updated.""" + def family_updated(self, handle_list: List[str]) -> None: + """ + Called when a family is updated. + + Args: + handle_list: List of family handles that were updated. + """ if self.active_family_handle in handle_list: self.collect_events() if self.drawing_area: self.drawing_area.queue_draw() - def person_updated(self, handle_list): - """Called when a person is updated.""" + def person_updated(self, handle_list: List[str]) -> None: + """ + Called when a person is updated. + + Args: + handle_list: List of person handles that were updated. + """ # Check if any updated person is related to current family if self.active_family_handle: try: @@ -307,20 +353,30 @@ class MyTimelineView(NavigationView): self.collect_events() if self.drawing_area: self.drawing_area.queue_draw() - except Exception: + except (AttributeError, KeyError) as e: # Skip if family handle is invalid - pass + logger.warning(f"Error accessing family in person_updated: {e}", exc_info=True) - def event_updated(self, handle_list): - """Called when an event is updated.""" + def event_updated(self, handle_list: List[str]) -> None: + """ + Called when an event is updated. + + Args: + handle_list: List of event handles that were updated. + """ # Re-collect events if we have an active family if self.active_family_handle: self.collect_events() if self.drawing_area: self.drawing_area.queue_draw() - def change_db(self, db): - """Called when the database changes.""" + def change_db(self, db) -> None: + """ + Called when the database changes. + + Args: + db: The new database object. + """ self.active_family_handle = None self.events = [] @@ -333,8 +389,13 @@ class MyTimelineView(NavigationView): if self.drawing_area: self.drawing_area.queue_draw() - def build_widget(self): - """Build the interface and return the container.""" + def build_widget(self) -> Gtk.Widget: + """ + Build the interface and return the container. + + Returns: + Gtk.Widget: The main container widget with toolbar and drawing area. + """ # Main container main_box = Gtk.Box(orientation=Gtk.Orientation.VERTICAL, spacing=5) @@ -400,14 +461,21 @@ class MyTimelineView(NavigationView): return main_box - def build_tree(self): - """Rebuilds the current display. Called when the view becomes visible.""" + def build_tree(self) -> None: + """ + Rebuilds the current display. Called when the view becomes visible. + """ active_handle = self.get_active() if active_handle: self.goto_handle(active_handle) - def goto_handle(self, handle): - """Called when the active family changes.""" + def goto_handle(self, handle: str) -> None: + """ + Called when the active family changes. + + Args: + handle: The handle of the family to display. + """ if handle == self.active_family_handle: return @@ -416,7 +484,70 @@ class MyTimelineView(NavigationView): if self.drawing_area: self.drawing_area.queue_draw() - def collect_events(self): + def _collect_person_event_refs(self, person) -> List: + """ + Collect event references from a person. + + Args: + person: The person object to collect events from. + + Returns: + List: List of event references. + """ + if not person: + return [] + try: + return person.get_event_ref_list() + except (AttributeError, KeyError) as e: + logger.warning(f"Error accessing event references for person: {e}", exc_info=True) + return [] + + def _process_event_ref(self, event_ref, person_obj: Optional[Any]) -> Optional[TimelineEvent]: + """ + Process a single event reference and create a TimelineEvent. + + Args: + event_ref: The event reference to process. + person_obj: The person object associated with this event (None for family events). + + Returns: + Optional[TimelineEvent]: The created TimelineEvent, or None if invalid. + """ + try: + event = self.dbstate.db.get_event_from_handle(event_ref.ref) + if event and event.get_date_object(): + date_obj = event.get_date_object() + event_type = event.get_type() + return TimelineEvent( + date_sort=date_obj.get_sort_value(), + date_obj=date_obj, + event=event, + person=person_obj, + event_type=event_type, + y_pos=0.0 + ) + except (AttributeError, KeyError, ValueError) as e: + logger.debug(f"Skipping invalid event reference: {e}") + return None + + def _collect_person_events(self, person, person_obj) -> None: + """ + Collect all events from a person and add them to self.events. + + Args: + person: The person object to collect events from. + person_obj: The person object to associate with events. + """ + if not person: + return + + event_refs = self._collect_person_event_refs(person) + for event_ref in event_refs: + timeline_event = self._process_event_ref(event_ref, person_obj) + if timeline_event: + self.events.append(timeline_event) + + def collect_events(self) -> None: """Collect all events for the active family.""" self.events = [] @@ -425,8 +556,8 @@ class MyTimelineView(NavigationView): try: family = self.dbstate.db.get_family_from_handle(self.active_family_handle) - except Exception: - # Database error - family handle may be invalid + except (AttributeError, KeyError) as e: + logger.warning(f"Error accessing family: {e}", exc_info=True) return if not family: @@ -434,54 +565,9 @@ class MyTimelineView(NavigationView): # Get family events (marriage, divorce, etc.) for event_ref in family.get_event_ref_list(): - try: - event = self.dbstate.db.get_event_from_handle(event_ref.ref) - if event and event.get_date_object(): - date_obj = event.get_date_object() - event_type = event.get_type() - self.events.append( - ( - date_obj.get_sort_value(), - date_obj, - event, - None, # No person for family events - event_type, - 0, # y_pos (will be calculated during draw) - ) - ) - except Exception: - # Skip invalid event references - pass - - # Helper function to collect all events from a person - def collect_person_events(person, person_obj): - """Collect all events from a person.""" - if not person: - return - try: - # Get all event references from the person - for event_ref in person.get_event_ref_list(): - try: - event = self.dbstate.db.get_event_from_handle(event_ref.ref) - if event and event.get_date_object(): - date_obj = event.get_date_object() - event_type = event.get_type() - self.events.append( - ( - date_obj.get_sort_value(), - date_obj, - event, - person_obj, - event_type, - 0, # y_pos - ) - ) - except Exception: - # Skip invalid event references - pass - except Exception: - # Skip if person has no events or error accessing events - pass + timeline_event = self._process_event_ref(event_ref, None) + if timeline_event: + self.events.append(timeline_event) # Get father's events father_handle = family.get_father_handle() @@ -489,10 +575,9 @@ class MyTimelineView(NavigationView): try: father = self.dbstate.db.get_person_from_handle(father_handle) if father: - collect_person_events(father, father) - except Exception: - # Skip if father handle is invalid - pass + self._collect_person_events(father, father) + except (AttributeError, KeyError) as e: + logger.debug(f"Error accessing father: {e}") # Get mother's events mother_handle = family.get_mother_handle() @@ -500,10 +585,9 @@ class MyTimelineView(NavigationView): try: mother = self.dbstate.db.get_person_from_handle(mother_handle) if mother: - collect_person_events(mother, mother) - except Exception: - # Skip if mother handle is invalid - pass + self._collect_person_events(mother, mother) + except (AttributeError, KeyError) as e: + logger.debug(f"Error accessing mother: {e}") # Get children's events for child_ref in family.get_child_ref_list(): @@ -511,16 +595,19 @@ class MyTimelineView(NavigationView): try: child = self.dbstate.db.get_person_from_handle(child_handle) if child: - collect_person_events(child, child) - except Exception: - # Skip if child handle is invalid - pass + self._collect_person_events(child, child) + except (AttributeError, KeyError) as e: + logger.debug(f"Error accessing child: {e}") # Sort events by date - self.events.sort(key=lambda x: x[0]) + self.events.sort(key=lambda x: x.date_sort) # Invalidate cache when events change self._adjusted_events_cache = None + self._cache_key = None + self._cached_date_range = None + self._cached_min_date = None + self._cached_max_date = None # Calculate timeline height based on number of events and zoom if self.events: @@ -536,8 +623,62 @@ class MyTimelineView(NavigationView): if self.drawing_area: self.drawing_area.set_size_request(800, self.timeline_height) - def _get_event_label_text(self, event, person, event_type): - """Generate label text for an event. Centralized logic.""" + def _calculate_date_range(self) -> Tuple[int, int, int]: + """ + Calculate date range from events. + + Returns: + Tuple[int, int, int]: (min_date, max_date, date_range) + """ + if not self.events: + return (0, 0, 1) + + if (self._cached_min_date is not None and + self._cached_max_date is not None and + self._cached_date_range is not None): + return (self._cached_min_date, self._cached_max_date, self._cached_date_range) + + min_date = min(event.date_sort for event in self.events) + max_date = max(event.date_sort for event in self.events) + date_range = max_date - min_date if max_date != min_date else 1 + + self._cached_min_date = min_date + self._cached_max_date = max_date + self._cached_date_range = date_range + + return (min_date, max_date, date_range) + + def _calculate_y_position(self, date_sort: int, min_date: int, date_range: int, + timeline_y_start: float, timeline_y_end: float) -> float: + """ + Calculate Y position for an event based on date. + + Args: + date_sort: The sort value of the event date. + min_date: The minimum date sort value. + date_range: The range of dates (max - min). + timeline_y_start: The Y coordinate of the timeline start. + timeline_y_end: The Y coordinate of the timeline end. + + Returns: + float: The calculated Y position. + """ + return timeline_y_start + ( + (date_sort - min_date) / date_range + ) * (timeline_y_end - timeline_y_start) + + def _get_event_label_text(self, event, person: Optional[Any], event_type: EventType) -> str: + """ + Generate label text for an event. Centralized logic. + + Args: + event: The event object. + person: The person associated with the event (None for family events). + event_type: The type of event. + + Returns: + str: The formatted label text. + """ date_str = get_date(event) event_type_str = str(event_type) @@ -547,8 +688,23 @@ class MyTimelineView(NavigationView): else: return f"{date_str} - {event_type_str}" - def _calculate_adjusted_positions(self, context, events_with_y_pos, timeline_y_start, timeline_y_end): - """Calculate adjusted Y positions with collision detection. Reusable by both drawing and click detection.""" + def _calculate_adjusted_positions(self, context: cairo.Context, + events_with_y_pos: List[TimelineEvent], + timeline_y_start: float, + timeline_y_end: float) -> List[TimelineEvent]: + """ + Calculate adjusted Y positions with collision detection. + Reusable by both drawing and click detection. + + Args: + context: Cairo drawing context for text measurement. + events_with_y_pos: List of TimelineEvent objects with initial Y positions. + timeline_y_start: The Y coordinate of the timeline start. + timeline_y_end: The Y coordinate of the timeline end. + + Returns: + List[TimelineEvent]: List of TimelineEvent objects with adjusted Y positions. + """ if not events_with_y_pos: return events_with_y_pos @@ -559,38 +715,59 @@ class MyTimelineView(NavigationView): adjusted_events = [] for event_data in events_with_y_pos: - date_sort, date_obj, event, person, event_type, y_pos = event_data - # Calculate label height using centralized text generation - label_text = self._get_event_label_text(event, person, event_type) + label_text = self._get_event_label_text(event_data.event, event_data.person, event_data.event_type) layout.set_text(label_text, -1) text_width, text_height = layout.get_pixel_size() label_height = text_height + LABEL_PADDING # Check for overlap with previous events - adjusted_y = y_pos - for prev_data in adjusted_events: - prev_y_pos = prev_data[5] # Get y_pos from previous event (index 5 now) + adjusted_y = event_data.y_pos + for prev_event in adjusted_events: # Check if labels would overlap - if abs(adjusted_y - prev_y_pos) < MIN_LABEL_SPACING: + if abs(adjusted_y - prev_event.y_pos) < MIN_LABEL_SPACING: # Adjust downward - adjusted_y = prev_y_pos + MIN_LABEL_SPACING + adjusted_y = prev_event.y_pos + MIN_LABEL_SPACING # Ensure adjusted position is within bounds adjusted_y = max(timeline_y_start, min(adjusted_y, timeline_y_end)) # Create new event data with adjusted Y position - adjusted_events.append((date_sort, date_obj, event, person, event_type, adjusted_y)) + adjusted_event = TimelineEvent( + date_sort=event_data.date_sort, + date_obj=event_data.date_obj, + event=event_data.event, + person=event_data.person, + event_type=event_data.event_type, + y_pos=adjusted_y + ) + adjusted_events.append(adjusted_event) return adjusted_events - def detect_label_overlaps(self, context, events_with_y_pos, timeline_y_start, timeline_y_end): - """Detect and adjust Y positions to prevent label overlaps.""" + def detect_label_overlaps(self, context: cairo.Context, + events_with_y_pos: List[TimelineEvent], + timeline_y_start: float, + timeline_y_end: float) -> List[TimelineEvent]: + """ + Detect and adjust Y positions to prevent label overlaps. + + Args: + context: Cairo drawing context. + events_with_y_pos: List of TimelineEvent objects with initial Y positions. + timeline_y_start: The Y coordinate of the timeline start. + timeline_y_end: The Y coordinate of the timeline end. + + Returns: + List[TimelineEvent]: List of TimelineEvent objects with adjusted Y positions. + """ # Use shared collision detection method return self._calculate_adjusted_positions(context, events_with_y_pos, timeline_y_start, timeline_y_end) - def _recalculate_timeline_height(self): - """Recalculate timeline height based on current events and zoom level.""" + def _recalculate_timeline_height(self) -> None: + """ + Recalculate timeline height based on current events and zoom level. + """ if self.events: base_height = ( TIMELINE_MARGIN_TOP @@ -606,9 +783,15 @@ class MyTimelineView(NavigationView): # Invalidate cache when zoom changes self._adjusted_events_cache = None + self._cache_key = None - def on_zoom_in(self, widget): - """Zoom in.""" + def on_zoom_in(self, widget: Gtk.Widget) -> None: + """ + Zoom in. + + Args: + widget: The widget that triggered the zoom (unused). + """ if self.zoom_level < self.max_zoom: self.zoom_level = min(self.zoom_level + self.zoom_step, self.max_zoom) self.update_zoom_display() @@ -616,8 +799,13 @@ class MyTimelineView(NavigationView): if self.drawing_area: self.drawing_area.queue_draw() - def on_zoom_out(self, widget): - """Zoom out.""" + def on_zoom_out(self, widget: Gtk.Widget) -> None: + """ + Zoom out. + + Args: + widget: The widget that triggered the zoom (unused). + """ if self.zoom_level > self.min_zoom: self.zoom_level = max(self.zoom_level - self.zoom_step, self.min_zoom) self.update_zoom_display() @@ -625,16 +813,30 @@ class MyTimelineView(NavigationView): if self.drawing_area: self.drawing_area.queue_draw() - def on_zoom_reset(self, widget): - """Reset zoom to 100%.""" + def on_zoom_reset(self, widget: Gtk.Widget) -> None: + """ + Reset zoom to 100%. + + Args: + widget: The widget that triggered the reset (unused). + """ self.zoom_level = 1.0 self.update_zoom_display() self._recalculate_timeline_height() # Only recalculate height, not events if self.drawing_area: self.drawing_area.queue_draw() - def on_scroll(self, widget, event): - """Handle scroll events for zooming with Ctrl+scroll.""" + def on_scroll(self, widget: Gtk.Widget, event: Gdk.Event) -> bool: + """ + Handle scroll events for zooming with Ctrl+scroll. + + Args: + widget: The widget that received the scroll event. + event: The scroll event. + + Returns: + bool: True if the event was handled, False otherwise. + """ if event.state & Gdk.ModifierType.CONTROL_MASK: if event.direction == Gdk.ScrollDirection.UP: self.on_zoom_in(widget) @@ -643,25 +845,31 @@ class MyTimelineView(NavigationView): return True return False - def update_zoom_display(self): + def update_zoom_display(self) -> None: """Update the zoom level display.""" if hasattr(self, 'zoom_label'): self.zoom_label.set_text(f"{int(self.zoom_level * 100)}%") - def on_button_press(self, widget, event): - """Handle mouse button press events.""" + def on_button_press(self, widget: Gtk.Widget, event: Gdk.Event) -> bool: + """ + Handle mouse button press events. + + Args: + widget: The widget that received the button press event. + event: The button press event. + + Returns: + bool: False to allow other handlers to process the event. + """ if event.button == 1: # Left click # Find which event was clicked clicked_index = self.find_event_at_position(event.x, event.y) if clicked_index is not None: - # Convert mouse coordinates to drawing coordinates (account for zoom) - scaled_x = event.x / self.zoom_level - - date_sort, date_obj, clicked_event, clicked_person, event_type, _y_pos = self.events[clicked_index] + clicked_event_data = self.events[clicked_index] # Clicking anywhere on the event line selects the person (like selecting the event) - if clicked_person: - person_handle = clicked_person.get_handle() + if clicked_event_data.person: + person_handle = clicked_event_data.person.get_handle() if self.selected_person_handle == person_handle: # Deselect if clicking same person self.selected_person_handle = None @@ -675,8 +883,17 @@ class MyTimelineView(NavigationView): self.drawing_area.queue_draw() return False - def on_motion_notify(self, widget, event): - """Handle mouse motion events for hover detection.""" + def on_motion_notify(self, widget: Gtk.Widget, event: Gdk.Event) -> bool: + """ + Handle mouse motion events for hover detection. + + Args: + widget: The widget that received the motion event. + event: The motion event. + + Returns: + bool: False to allow other handlers to process the event. + """ self.mouse_x = event.x self.mouse_y = event.y @@ -706,8 +923,17 @@ class MyTimelineView(NavigationView): return False - def on_leave_notify(self, widget, event): - """Handle mouse leave events.""" + def on_leave_notify(self, widget: Gtk.Widget, event: Gdk.Event) -> bool: + """ + Handle mouse leave events. + + Args: + widget: The widget that received the leave event. + event: The leave event. + + Returns: + bool: False to allow other handlers to process the event. + """ self.hovered_event_index = None # Cancel tooltip timeout @@ -723,47 +949,84 @@ class MyTimelineView(NavigationView): self.drawing_area.queue_draw() return False - def _get_adjusted_events(self, context, timeline_y_start, timeline_y_end): - """Get adjusted events with collision detection, using cache if available.""" - # Check if cache is valid + def _get_cache_key(self, timeline_y_start: float, timeline_y_end: float) -> int: + """ + Generate cache key for adjusted events. + + Args: + timeline_y_start: The Y coordinate of the timeline start. + timeline_y_end: The Y coordinate of the timeline end. + + Returns: + int: A hash-based cache key. + """ + return hash((len(self.events), timeline_y_start, timeline_y_end)) + + def _get_adjusted_events(self, context: cairo.Context, + timeline_y_start: float, + timeline_y_end: float) -> List[TimelineEvent]: + """ + Get adjusted events with collision detection, using cache if available. + + Args: + context: Cairo drawing context for text measurement. + timeline_y_start: The Y coordinate of the timeline start. + timeline_y_end: The Y coordinate of the timeline end. + + Returns: + List[TimelineEvent]: List of TimelineEvent objects with adjusted Y positions. + """ + # Check if cache is valid using hash-based key + cache_key = self._get_cache_key(timeline_y_start, timeline_y_end) if (self._adjusted_events_cache is not None and - self._cache_context == context and - self._cache_timeline_y_start == timeline_y_start and - self._cache_timeline_y_end == timeline_y_end): + self._cache_key == cache_key): return self._adjusted_events_cache # Calculate date range if not self.events: return [] - min_date = min(event[0] for event in self.events) - max_date = max(event[0] for event in self.events) - date_range = max_date - min_date - if date_range == 0: - date_range = 1 + min_date, max_date, date_range = self._calculate_date_range() # Calculate initial Y positions events_with_y_pos = [] for event_data in self.events: - date_sort, date_obj, event, person, event_type, _ = event_data - y_pos = timeline_y_start + ( - (date_sort - min_date) / date_range - ) * (timeline_y_end - timeline_y_start) - events_with_y_pos.append((date_sort, date_obj, event, person, event_type, y_pos)) + y_pos = self._calculate_y_position( + event_data.date_sort, min_date, date_range, + timeline_y_start, timeline_y_end + ) + event_with_y = TimelineEvent( + date_sort=event_data.date_sort, + date_obj=event_data.date_obj, + event=event_data.event, + person=event_data.person, + event_type=event_data.event_type, + y_pos=y_pos + ) + events_with_y_pos.append(event_with_y) # Apply collision detection using shared method - adjusted_events = self._calculate_adjusted_positions(context, events_with_y_pos, timeline_y_start, timeline_y_end) + adjusted_events = self._calculate_adjusted_positions( + context, events_with_y_pos, timeline_y_start, timeline_y_end + ) # Cache the results self._adjusted_events_cache = adjusted_events - self._cache_context = context - self._cache_timeline_y_start = timeline_y_start - self._cache_timeline_y_end = timeline_y_end + self._cache_key = cache_key return adjusted_events - def find_event_at_position(self, x, y): - """Find which event is at the given position.""" + def find_event_at_position(self, x: float, y: float) -> Optional[int]: + """ + Find which event is at the given position. + + Args: + x: X coordinate in widget space. + y: Y coordinate in widget space. + + Returns: + Optional[int]: Index of the event at the position, or None if no event found. + """ if not self.events or not self.drawing_area: return None @@ -791,92 +1054,141 @@ class MyTimelineView(NavigationView): label_x = timeline_x + LABEL_X_OFFSET for i, event_data in enumerate(adjusted_events): - date_sort, date_obj, event, person, event_type, adjusted_y = event_data - # Check if click is in the event's area (marker + label) if (scaled_x >= timeline_x - marker_size - MARKER_CLICK_PADDING and scaled_x <= label_x + CLICKABLE_AREA_WIDTH and - abs(scaled_y - adjusted_y) < CLICKABLE_AREA_HEIGHT / 2): + abs(scaled_y - event_data.y_pos) < CLICKABLE_AREA_HEIGHT / 2): return i return None - def show_tooltip(self, event_index, x_root, y_root): - """Show tooltip for an event, including all events for that person.""" - if event_index is None or event_index >= len(self.events): - return False + def _format_person_tooltip(self, person, person_events: List[TimelineEvent]) -> str: + """ + Format tooltip for person with multiple events. - date_sort, date_obj, event, person, event_type, y_pos = self.events[event_index] + Args: + person: The person object. + person_events: List of TimelineEvent objects for this person. + + Returns: + str: Formatted tooltip text. + """ + person_name = name_displayer.display(person) + tooltip_text = f"{person_name}\n" + tooltip_text += "─" * 30 + "\n" - # If event has a person, show all events for that person - if person: - person_handle = person.get_handle() - person_name = name_displayer.display(person) + # Sort by date + person_events_sorted = sorted(person_events, key=lambda x: x.date_sort) + + # List all events for this person (date first) + for event_data in person_events_sorted: + date_str = get_date(event_data.event) + event_type_str = str(event_data.event_type) + tooltip_text += f"{date_str} - {event_type_str}\n" - # Find all events for this person - person_events = [] - for evt_data in self.events: - evt_date_sort, evt_date_obj, evt_event, evt_person, evt_event_type, evt_y_pos = evt_data - if evt_person and evt_person.get_handle() == person_handle: - person_events.append((evt_date_sort, evt_date_obj, evt_event, evt_event_type)) - - # Sort by date - person_events.sort(key=lambda x: x[0]) - - # Build tooltip text with person name as header - tooltip_text = f"{person_name}\n" - tooltip_text += "─" * 30 + "\n" - - # List all events for this person (date first) - for evt_date_sort, evt_date_obj, evt_event, evt_event_type in person_events: - evt_date_str = get_date(evt_event) - evt_event_type_str = str(evt_event_type) - tooltip_text += f"{evt_date_str} - {evt_event_type_str}\n" - - # Add place if available - evt_place_handle = evt_event.get_place_handle() - if evt_place_handle: - try: - evt_place = self.dbstate.db.get_place_from_handle(evt_place_handle) - if evt_place: - evt_place_name = evt_place.get_title() - tooltip_text += f" 📍 {evt_place_name}\n" - except Exception: - # Skip if place handle is invalid - pass - else: - # Family event (no person) - show single event info (date first) - date_str = get_date(event) - event_type_str = str(event_type) - - tooltip_text = f"{date_str}\n{event_type_str}" - - # Get place information - place_handle = event.get_place_handle() + # Add place if available + place_handle = event_data.event.get_place_handle() if place_handle: try: place = self.dbstate.db.get_place_from_handle(place_handle) if place: - place_name = place.get_title() - tooltip_text += f"\n📍 {place_name}" - except Exception: - # Skip if place handle is invalid - pass + place_name = place.get_title() + tooltip_text += f" 📍 {place_name}\n" + except (AttributeError, KeyError) as e: + logger.debug(f"Error accessing place in tooltip: {e}") + + return tooltip_text + + def _format_single_event_tooltip(self, event, event_type: EventType) -> str: + """ + Format tooltip for single event. + + Args: + event: The event object. + event_type: The type of event. - # Get description - description = event.get_description() - if description: - tooltip_text += f"\n{description}" + Returns: + str: Formatted tooltip text. + """ + date_str = get_date(event) + event_type_str = str(event_type) - # Create tooltip window - self.tooltip_window = Gtk.Window(type=Gtk.WindowType.POPUP) - self.tooltip_window.set_border_width(8) - self.tooltip_window.set_decorated(False) + tooltip_text = f"{date_str}\n{event_type_str}" - # Get parent window for proper display - toplevel = self.drawing_area.get_toplevel() - if isinstance(toplevel, Gtk.Window): - self.tooltip_window.set_transient_for(toplevel) + # Get place information + place_handle = event.get_place_handle() + if place_handle: + try: + place = self.dbstate.db.get_place_from_handle(place_handle) + if place: + place_name = place.get_title() + tooltip_text += f"\n📍 {place_name}" + except (AttributeError, KeyError) as e: + logger.debug(f"Error accessing place in tooltip: {e}") + + # Get description + description = event.get_description() + if description: + tooltip_text += f"\n{description}" + + return tooltip_text + + def _get_or_create_tooltip_window(self) -> Gtk.Window: + """ + Get or create tooltip window (reuse if exists). + + Returns: + Gtk.Window: The tooltip window. + """ + 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_decorated(False) + + # Get parent window for proper display + toplevel = self.drawing_area.get_toplevel() + if isinstance(toplevel, Gtk.Window): + self.tooltip_window.set_transient_for(toplevel) + return self.tooltip_window + + def show_tooltip(self, event_index: int, x_root: float, y_root: float) -> bool: + """ + Show tooltip for an event, including all events for that person. + + Args: + event_index: Index of the event in self.events. + x_root: X coordinate in root window space. + y_root: Y coordinate in root window space. + + Returns: + bool: False to indicate the timeout should not be repeated. + """ + if event_index is None or event_index >= len(self.events): + return False + + event_data = self.events[event_index] + + # If event has a person, show all events for that person + if event_data.person: + person_handle = event_data.person.get_handle() + + # Find all events for this person + person_events = [ + evt for evt in self.events + if evt.person and evt.person.get_handle() == person_handle + ] + + tooltip_text = self._format_person_tooltip(event_data.person, person_events) + else: + # Family event (no person) - show single event info + tooltip_text = self._format_single_event_tooltip(event_data.event, event_data.event_type) + + # Get or create tooltip window + tooltip_window = self._get_or_create_tooltip_window() + + # Clear existing content + for child in tooltip_window.get_children(): + tooltip_window.remove(child) # Create container with background frame = Gtk.Frame() @@ -893,16 +1205,115 @@ class MyTimelineView(NavigationView): label.set_margin_bottom(5) frame.add(label) - self.tooltip_window.add(frame) - self.tooltip_window.show_all() + tooltip_window.add(frame) + tooltip_window.show_all() # Position tooltip (offset to avoid cursor) - self.tooltip_window.move(int(x_root) + 15, int(y_root) + 15) + tooltip_window.move(int(x_root) + 15, int(y_root) + 15) return False - def on_draw(self, widget, context): - """Draw the timeline.""" + def _draw_background(self, context: cairo.Context, width: float, height: float) -> None: + """ + Draw background gradient. + + Args: + context: Cairo drawing context. + width: Widget width. + 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 + context.set_source(pattern) + context.paint() + + def _draw_no_events_message(self, context: cairo.Context, width: float, height: float) -> None: + """ + Draw "No events" message. + + Args: + context: Cairo drawing context. + width: Widget width. + height: Widget height. + """ + context.set_source_rgb(0.6, 0.6, 0.6) + context.select_font_face("Sans", cairo.FONT_SLANT_NORMAL, cairo.FONT_WEIGHT_NORMAL) + context.set_font_size(24) + text = _("No events found") + (x_bearing, y_bearing, text_width, text_height, x_advance, y_advance) = context.text_extents(text) + context.move_to((width - text_width) / 2, height / 2) + context.show_text(text) + + def _draw_timeline_axis(self, context: cairo.Context, timeline_x: float, + y_start: float, y_end: float) -> None: + """ + Draw timeline axis with shadow. + + Args: + context: Cairo drawing context. + timeline_x: X coordinate of the timeline. + y_start: Y coordinate of the timeline start. + y_end: Y coordinate of the timeline end. + """ + # Draw shadow + context.set_source_rgba(0.0, 0.0, 0.0, 0.2) + 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.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) + context.set_source(pattern) + context.set_line_width(TIMELINE_LINE_WIDTH) + context.move_to(timeline_x, y_start) + context.line_to(timeline_x, y_end) + context.stroke() + + def _draw_events(self, context: cairo.Context, events_with_y_pos: List[TimelineEvent], + timeline_x: float) -> None: + """ + Draw all events. + + Args: + context: Cairo drawing context. + events_with_y_pos: List of TimelineEvent objects with Y positions. + timeline_x: X coordinate of the timeline. + """ + for i, event_data in enumerate(events_with_y_pos): + # Check if this event is hovered + is_hovered = (i == self.hovered_event_index) + + # Check if this event belongs to selected person + is_selected = (self.selected_person_handle is not None and + event_data.person and + event_data.person.get_handle() == self.selected_person_handle) + + # Draw event marker with modern styling + self.draw_event_marker(context, timeline_x, event_data.y_pos, + event_data.event_type, is_hovered, is_selected) + + # Draw event label + label_x = timeline_x + LABEL_X_OFFSET + self.draw_event_label( + context, label_x, event_data.y_pos, event_data.date_obj, + event_data.event, event_data.person, event_data.event_type, is_hovered + ) + + def on_draw(self, widget: Gtk.Widget, context: cairo.Context) -> bool: + """ + Draw the timeline. + + Args: + widget: The drawing area widget. + context: Cairo drawing context. + + Returns: + bool: True to indicate the draw was handled. + """ # Apply zoom transformation context.save() context.scale(self.zoom_level, self.zoom_level) @@ -911,88 +1322,97 @@ class MyTimelineView(NavigationView): width = widget.get_allocated_width() / self.zoom_level height = widget.get_allocated_height() / self.zoom_level - # Clear background with modern gradient - 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 - context.set_source(pattern) - context.paint() + # Draw background + self._draw_background(context, width, height) if not self.events: - # Draw "No events" message with modern styling - context.set_source_rgb(0.6, 0.6, 0.6) - context.select_font_face("Sans", cairo.FONT_SLANT_NORMAL, cairo.FONT_WEIGHT_NORMAL) - context.set_font_size(24) - text = _("No events found") - (x_bearing, y_bearing, text_width, text_height, x_advance, y_advance) = context.text_extents(text) - context.move_to((width - text_width) / 2, height / 2) - context.show_text(text) + # Draw "No events" message + self._draw_no_events_message(context, width, height) context.restore() - return + return True # Calculate date range - min_date = min(event[0] for event in self.events) - max_date = max(event[0] for event in self.events) - date_range = max_date - min_date - if date_range == 0: - date_range = 1 + min_date, max_date, date_range = self._calculate_date_range() - # Draw timeline axis with shadow + # Draw timeline axis timeline_x = TIMELINE_MARGIN_LEFT timeline_y_start = TIMELINE_MARGIN_TOP timeline_y_end = height - TIMELINE_MARGIN_BOTTOM - - # Draw shadow - context.set_source_rgba(0.0, 0.0, 0.0, 0.2) - context.set_line_width(TIMELINE_LINE_WIDTH) - context.move_to(timeline_x + 2, timeline_y_start + 2) - context.line_to(timeline_x + 2, timeline_y_end + 2) - context.stroke() - - # Draw main line with gradient - pattern = cairo.LinearGradient(timeline_x, timeline_y_start, timeline_x, timeline_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) - context.set_source(pattern) - context.set_line_width(TIMELINE_LINE_WIDTH) - context.move_to(timeline_x, timeline_y_start) - context.line_to(timeline_x, timeline_y_end) - context.stroke() + + self._draw_timeline_axis(context, timeline_x, timeline_y_start, timeline_y_end) # Get adjusted events with collision detection (uses cache) events_with_y_pos = self._get_adjusted_events(context, timeline_y_start, timeline_y_end) # Draw events - for i, event_data in enumerate(events_with_y_pos): - date_sort, date_obj, event, person, event_type, y_pos = event_data - - # Check if this event is hovered - is_hovered = (i == self.hovered_event_index) - - # Check if this event belongs to selected person - is_selected = (self.selected_person_handle is not None and - person and person.get_handle() == self.selected_person_handle) - - # Draw event marker with modern styling - self.draw_event_marker(context, timeline_x, y_pos, event_type, is_hovered, is_selected) - - # Draw event label - label_x = timeline_x + LABEL_X_OFFSET - self.draw_event_label( - context, label_x, y_pos, date_obj, event, person, event_type, is_hovered - ) + self._draw_events(context, events_with_y_pos, timeline_x) # Draw visual connections for selected person if self.selected_person_handle is not None: - self.draw_person_connections(context, events_with_y_pos, timeline_x, timeline_y_start, timeline_y_end) + self.draw_person_connections(context, events_with_y_pos, timeline_x, + timeline_y_start, timeline_y_end) # Draw year markers on the left - self.draw_year_markers(context, timeline_x, timeline_y_start, timeline_y_end, min_date, max_date) + self.draw_year_markers(context, timeline_x, timeline_y_start, timeline_y_end, + min_date, max_date) context.restore() + return True - def draw_event_marker(self, context, x, y, event_type, is_hovered=False, is_selected=False): - """Draw a marker for an event with modern styling.""" + def _draw_marker_shadow(self, context: cairo.Context, x: float, y: float, + size: float, shape: str) -> None: + """ + Draw shadow for marker. + + Args: + context: Cairo drawing context. + x: X coordinate of marker center. + y: Y coordinate of marker center. + size: Size of the marker. + shape: Shape type ('triangle', 'circle', etc.). + """ + context.set_source_rgba(0.0, 0.0, 0.0, 0.3) + context.translate(1, 1) + self._draw_shape(context, x, y, size, shape) + context.fill() + context.translate(-1, -1) + + def _draw_marker_gradient(self, context: cairo.Context, x: float, y: float, + size: float, color: Tuple[float, float, float], + shape: str) -> None: + """ + Draw gradient fill for marker. + + Args: + context: Cairo drawing context. + x: X coordinate of marker center. + y: Y coordinate of marker center. + size: Size of the marker. + color: RGB color tuple (0.0-1.0). + shape: Shape type ('triangle', 'circle', etc.). + """ + pattern = cairo.RadialGradient(x - size/2, y - size/2, 0, x, y, size) + r, g, b = color + pattern.add_color_stop_rgb(0, min(1.0, r + 0.2), min(1.0, g + 0.2), min(1.0, b + 0.2)) + pattern.add_color_stop_rgb(1, max(0.0, r - 0.1), max(0.0, g - 0.1), max(0.0, b - 0.1)) + context.set_source(pattern) + self._draw_shape(context, x, y, size, shape) + context.fill() + + def draw_event_marker(self, context: cairo.Context, x: float, y: float, + event_type: EventType, is_hovered: bool = False, + is_selected: bool = False) -> None: + """ + Draw a marker for an event with modern styling. + + Args: + context: Cairo drawing context. + x: X coordinate for marker center. + y: Y coordinate for marker center. + event_type: Type of event (determines color and shape). + is_hovered: Whether marker is currently hovered. + is_selected: Whether marker belongs to selected person. + """ context.save() # Get integer value from EventType object for dictionary lookup @@ -1014,21 +1434,10 @@ class MyTimelineView(NavigationView): color = (0.2, 0.4, 0.9) # Blue highlight for selected person's events # Draw shadow - context.set_source_rgba(0.0, 0.0, 0.0, 0.3) - context.translate(1, 1) - self._draw_shape(context, x, y, marker_size, shape) - context.fill() - context.translate(-1, -1) + self._draw_marker_shadow(context, x, y, marker_size, shape) # Draw main shape with gradient - pattern = cairo.RadialGradient(x - marker_size/2, y - marker_size/2, 0, x, y, marker_size) - r, g, b = color - pattern.add_color_stop_rgb(0, min(1.0, r + 0.2), min(1.0, g + 0.2), min(1.0, b + 0.2)) - pattern.add_color_stop_rgb(1, max(0.0, r - 0.1), max(0.0, g - 0.1), max(0.0, b - 0.1)) - context.set_source(pattern) - - self._draw_shape(context, x, y, marker_size, shape) - context.fill() + self._draw_marker_gradient(context, x, y, marker_size, color, shape) # Draw border context.set_source_rgba(0.0, 0.0, 0.0, 0.3) @@ -1038,8 +1447,18 @@ class MyTimelineView(NavigationView): context.restore() - def _draw_shape(self, context, x, y, size, shape): - """Draw a shape at the given position.""" + def _draw_shape(self, context: cairo.Context, x: float, y: float, + size: float, shape: str) -> None: + """ + Draw a shape at the given position. + + Args: + context: Cairo drawing context. + x: X coordinate of shape center. + y: Y coordinate of shape center. + size: Size of the shape. + shape: Shape type ('triangle', 'circle', 'diamond', 'square', 'star', 'hexagon'). + """ if shape == 'triangle': context.move_to(x, y - size) context.line_to(x - size, y + size) @@ -1085,8 +1504,22 @@ class MyTimelineView(NavigationView): context.line_to(px, py) context.close_path() - def draw_event_label(self, context, x, y, date_obj, event, person, event_type, is_hovered=False): - """Draw the label for an event with modern styling.""" + def draw_event_label(self, context: cairo.Context, x: float, y: float, + date_obj: Date, event, person: Optional[Any], + event_type: EventType, is_hovered: bool = False) -> None: + """ + Draw the label for an event with modern styling. + + Args: + context: Cairo drawing context. + x: X coordinate for label start. + y: Y coordinate for label center. + date_obj: Date object for the event. + event: The event object. + person: The person associated with the event (None for family events). + event_type: The type of event. + is_hovered: Whether the label is currently hovered. + """ context.save() # Create Pango layout for text @@ -1107,7 +1540,7 @@ class MyTimelineView(NavigationView): # Draw background for hovered events if is_hovered: text_width, text_height = layout.get_pixel_size() - padding = 8 + padding = LABEL_BACKGROUND_PADDING # Draw rounded rectangle background rect_x = x - padding @@ -1116,7 +1549,7 @@ class MyTimelineView(NavigationView): rect_height = text_height + padding * 2 # Rounded rectangle - radius = 5 + radius = LABEL_BACKGROUND_RADIUS context.arc(rect_x + radius, rect_y + radius, radius, math.pi, 3 * math.pi / 2) context.arc(rect_x + rect_width - radius, rect_y + radius, radius, 3 * math.pi / 2, 0) context.arc(rect_x + rect_width - radius, rect_y + rect_height - radius, radius, 0, math.pi / 2) @@ -1139,24 +1572,35 @@ class MyTimelineView(NavigationView): context.restore() - def draw_year_markers(self, context, timeline_x, y_start, y_end, min_date, max_date): - """Draw year markers on the left side of the timeline.""" + def draw_year_markers(self, context: cairo.Context, timeline_x: float, + y_start: float, y_end: float, min_date: int, + max_date: int) -> None: + """ + Draw year markers on the left side of the timeline. + + Args: + context: Cairo drawing context. + timeline_x: X coordinate of the timeline. + y_start: Y coordinate of the timeline start. + y_end: Y coordinate of the timeline end. + min_date: Minimum date sort value. + max_date: Maximum date sort value. + """ context.save() # Find min and max years from events min_year = None max_year = None - for date_sort, date_obj, event, person, event_type, y_pos in self.events: + for event_data in self.events: try: - year = date_obj.get_year() + year = event_data.date_obj.get_year() if year and year != 0: if min_year is None or year < min_year: min_year = year if max_year is None or year > max_year: max_year = year - except Exception: - # Skip if date object is invalid - pass + except (AttributeError, ValueError) as e: + logger.debug(f"Error accessing year from date: {e}") if min_year is None or max_year is None: context.restore() @@ -1208,23 +1652,35 @@ class MyTimelineView(NavigationView): context.restore() - def draw_person_connections(self, context, events_with_y_pos, timeline_x, timeline_y_start, timeline_y_end): - """Draw visual connections between all events of the selected person.""" + def draw_person_connections(self, context: cairo.Context, + events_with_y_pos: List[TimelineEvent], + timeline_x: float, timeline_y_start: float, + timeline_y_end: float) -> None: + """ + Draw visual connections between all events of the selected person. + + Args: + context: Cairo drawing context. + events_with_y_pos: List of TimelineEvent objects with Y positions. + timeline_x: X coordinate of the timeline. + timeline_y_start: Y coordinate of the timeline start. + timeline_y_end: Y coordinate of the timeline end. + """ if not self.selected_person_handle: return # Find all events for the selected person - person_events = [] - for event_data in events_with_y_pos: - date_sort, date_obj, event, person, event_type, y_pos = event_data - if person and person.get_handle() == self.selected_person_handle: - person_events.append((y_pos, event_data)) + person_events = [ + event_data for event_data in events_with_y_pos + if event_data.person and + event_data.person.get_handle() == self.selected_person_handle + ] if len(person_events) < 1: return # Sort by Y position - person_events.sort(key=lambda x: x[0]) + person_events.sort(key=lambda x: x.y_pos) context.save() @@ -1241,7 +1697,7 @@ class MyTimelineView(NavigationView): # Draw vertical connector line on the left side if len(person_events) > 1: - y_positions = [y for y, _event_data in person_events] + y_positions = [event_data.y_pos for event_data in person_events] min_y = min(y_positions) max_y = max(y_positions) @@ -1256,14 +1712,19 @@ class MyTimelineView(NavigationView): # Draw horizontal lines connecting vertical line to each event marker context.set_source_rgba(0.2, 0.5, 1.0, 0.75) # Brighter, more opaque blue context.set_line_width(3.5) - for y_pos, event_data in person_events: + for event_data in person_events: # Draw horizontal line from vertical line to event marker - context.move_to(vertical_line_x, y_pos) - context.line_to(timeline_x, y_pos) + context.move_to(vertical_line_x, event_data.y_pos) + context.line_to(timeline_x, event_data.y_pos) context.stroke() context.restore() - def get_stock(self): - """Return the stock icon name.""" + def get_stock(self) -> str: + """ + Return the stock icon name. + + Returns: + str: The stock icon name for this view. + """ return "gramps-family"