Refactor: Extract constants and optimize code quality
- Use _normalize_event_type() helper consistently in draw_event_marker() - Extract label drawing constants (text color, vertical offset, hover colors) - Extract marker border width and black color constants - Extract year marker label offset constant - Create _get_events_for_person() helper to reduce duplication - Replace len() < 1 with 'not' for better Pythonic code - Optimize y_positions min/max using sorted list indices - Extract connection line minimum distance constant These changes improve code maintainability, reduce duplication, and enhance readability by replacing magic numbers with named constants.
This commit is contained in:
parent
bd8d94d1b5
commit
72bce841f4
@ -150,6 +150,7 @@ CONNECTION_LINE_WIDTH = 3.5
|
|||||||
CONNECTION_VERTICAL_LINE_X = 5 # Left of year markers
|
CONNECTION_VERTICAL_LINE_X = 5 # Left of year markers
|
||||||
CONNECTION_LINE_SPACING = 25 # Pixels between vertical lines for different persons
|
CONNECTION_LINE_SPACING = 25 # Pixels between vertical lines for different persons
|
||||||
TIMELINE_LEFT_SPACING = 20 # Spacing between rightmost connection line and timeline
|
TIMELINE_LEFT_SPACING = 20 # Spacing between rightmost connection line and timeline
|
||||||
|
CONNECTION_LINE_MIN_DISTANCE = EVENT_MARKER_SIZE * 2 # Minimum distance between events to draw vertical connector line
|
||||||
|
|
||||||
# Color Generation Constants
|
# Color Generation Constants
|
||||||
PERSON_COLOR_SATURATION = 0.7 # High saturation for vibrant colors
|
PERSON_COLOR_SATURATION = 0.7 # High saturation for vibrant colors
|
||||||
@ -162,6 +163,19 @@ SELECTED_MARKER_COLOR = (0.2, 0.4, 0.9) # Blue highlight for selected person's
|
|||||||
# Default Colors
|
# Default Colors
|
||||||
DEFAULT_EVENT_COLOR = (0.5, 0.5, 0.5) # Gray for unknown event types
|
DEFAULT_EVENT_COLOR = (0.5, 0.5, 0.5) # Gray for unknown event types
|
||||||
YEAR_MARKER_COLOR = (0.5, 0.5, 0.5) # Gray for year marker tick marks
|
YEAR_MARKER_COLOR = (0.5, 0.5, 0.5) # Gray for year marker tick marks
|
||||||
|
COLOR_BLACK = (0.0, 0.0, 0.0) # Black color used in multiple drawing operations
|
||||||
|
|
||||||
|
# Label Drawing Constants
|
||||||
|
LABEL_TEXT_COLOR = (0.1, 0.1, 0.1) # Dark gray text color for event labels
|
||||||
|
LABEL_VERTICAL_OFFSET = 10 # Vertical offset to center label on marker
|
||||||
|
LABEL_HOVER_BACKGROUND_COLOR = (1.0, 1.0, 1.0, 0.8) # Semi-transparent white background for hovered labels
|
||||||
|
LABEL_HOVER_BORDER_COLOR = (0.7, 0.7, 0.7, 0.5) # Light gray border for hovered labels
|
||||||
|
|
||||||
|
# Marker Drawing Constants
|
||||||
|
MARKER_BORDER_WIDTH = 1 # Line width for marker borders
|
||||||
|
|
||||||
|
# Year Marker Constants
|
||||||
|
YEAR_MARKER_LABEL_OFFSET = 20 # Horizontal offset for year marker labels from timeline
|
||||||
|
|
||||||
# Logger
|
# Logger
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@ -2104,6 +2118,22 @@ class MyTimelineView(NavigationView):
|
|||||||
return True
|
return True
|
||||||
return person_handle in self.person_filter
|
return person_handle in self.person_filter
|
||||||
|
|
||||||
|
def _get_events_for_person(self, person_handle: str, events_list: List['TimelineEvent']) -> List['TimelineEvent']:
|
||||||
|
"""
|
||||||
|
Filter events list to only include events for a specific person.
|
||||||
|
|
||||||
|
Args:
|
||||||
|
person_handle: The handle of the person to filter events for.
|
||||||
|
events_list: List of timeline events to filter.
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
List[TimelineEvent]: Filtered list of events for the specified person.
|
||||||
|
"""
|
||||||
|
return [
|
||||||
|
event for event in events_list
|
||||||
|
if self._person_matches_handle(event.person, person_handle)
|
||||||
|
]
|
||||||
|
|
||||||
def _get_event_type_display_name(self, event_type: EventType) -> str:
|
def _get_event_type_display_name(self, event_type: EventType) -> str:
|
||||||
"""
|
"""
|
||||||
Get human-readable display name for an event type.
|
Get human-readable display name for an event type.
|
||||||
@ -2828,10 +2858,7 @@ class MyTimelineView(NavigationView):
|
|||||||
person_handle = event_data.person.get_handle()
|
person_handle = event_data.person.get_handle()
|
||||||
|
|
||||||
# Find all events for this person
|
# Find all events for this person
|
||||||
person_events = [
|
person_events = self._get_events_for_person(person_handle, self.events)
|
||||||
evt for evt in self.events
|
|
||||||
if self._person_matches_handle(evt.person, person_handle)
|
|
||||||
]
|
|
||||||
|
|
||||||
tooltip_text = self._format_person_tooltip(event_data.person, person_events)
|
tooltip_text = self._format_person_tooltip(event_data.person, person_events)
|
||||||
else:
|
else:
|
||||||
@ -2912,7 +2939,7 @@ 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, TIMELINE_SHADOW_OPACITY)
|
context.set_source_rgba(*COLOR_BLACK, TIMELINE_SHADOW_OPACITY)
|
||||||
context.set_line_width(TIMELINE_LINE_WIDTH)
|
context.set_line_width(TIMELINE_LINE_WIDTH)
|
||||||
context.move_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_start + TIMELINE_SHADOW_OFFSET)
|
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.line_to(timeline_x + TIMELINE_SHADOW_OFFSET, y_end + TIMELINE_SHADOW_OFFSET)
|
||||||
@ -3025,7 +3052,7 @@ class MyTimelineView(NavigationView):
|
|||||||
size: Size of the marker.
|
size: Size of the marker.
|
||||||
shape: Shape type ('triangle', 'circle', etc.).
|
shape: Shape type ('triangle', 'circle', etc.).
|
||||||
"""
|
"""
|
||||||
context.set_source_rgba(0.0, 0.0, 0.0, SHADOW_OPACITY)
|
context.set_source_rgba(*COLOR_BLACK, SHADOW_OPACITY)
|
||||||
context.translate(SHADOW_OFFSET_X, SHADOW_OFFSET_Y)
|
context.translate(SHADOW_OFFSET_X, SHADOW_OFFSET_Y)
|
||||||
self._draw_shape(context, x, y, size, shape)
|
self._draw_shape(context, x, y, size, shape)
|
||||||
context.fill()
|
context.fill()
|
||||||
@ -3076,7 +3103,7 @@ class MyTimelineView(NavigationView):
|
|||||||
context.save()
|
context.save()
|
||||||
|
|
||||||
# Get integer value from EventType object for dictionary lookup
|
# Get integer value from EventType object for dictionary lookup
|
||||||
event_type_value = event_type.value if hasattr(event_type, 'value') else int(event_type)
|
event_type_value = self._normalize_event_type(event_type)
|
||||||
|
|
||||||
# Get color and shape
|
# Get color and shape
|
||||||
color = EVENT_COLORS.get(event_type_value, DEFAULT_EVENT_COLOR)
|
color = EVENT_COLORS.get(event_type_value, DEFAULT_EVENT_COLOR)
|
||||||
@ -3100,8 +3127,8 @@ class MyTimelineView(NavigationView):
|
|||||||
self._draw_marker_gradient(context, x, y, marker_size, color, shape)
|
self._draw_marker_gradient(context, x, y, marker_size, color, shape)
|
||||||
|
|
||||||
# Draw border
|
# Draw border
|
||||||
context.set_source_rgba(0.0, 0.0, 0.0, BORDER_OPACITY)
|
context.set_source_rgba(*COLOR_BLACK, BORDER_OPACITY)
|
||||||
context.set_line_width(1)
|
context.set_line_width(MARKER_BORDER_WIDTH)
|
||||||
self._draw_shape(context, x, y, marker_size, shape)
|
self._draw_shape(context, x, y, marker_size, shape)
|
||||||
context.stroke()
|
context.stroke()
|
||||||
|
|
||||||
@ -3235,17 +3262,17 @@ class MyTimelineView(NavigationView):
|
|||||||
context.close_path()
|
context.close_path()
|
||||||
|
|
||||||
# Fill with semi-transparent background
|
# Fill with semi-transparent background
|
||||||
context.set_source_rgba(1.0, 1.0, 1.0, 0.8)
|
context.set_source_rgba(*LABEL_HOVER_BACKGROUND_COLOR)
|
||||||
context.fill()
|
context.fill()
|
||||||
|
|
||||||
# Draw border
|
# Draw border
|
||||||
context.set_source_rgba(0.7, 0.7, 0.7, 0.5)
|
context.set_source_rgba(*LABEL_HOVER_BORDER_COLOR)
|
||||||
context.set_line_width(1)
|
context.set_line_width(MARKER_BORDER_WIDTH)
|
||||||
context.stroke()
|
context.stroke()
|
||||||
|
|
||||||
# Draw text
|
# Draw text
|
||||||
context.set_source_rgb(0.1, 0.1, 0.1) # Dark gray text
|
context.set_source_rgb(*LABEL_TEXT_COLOR)
|
||||||
context.move_to(x, y - 10) # Center vertically on marker
|
context.move_to(x, y - LABEL_VERTICAL_OFFSET)
|
||||||
PangoCairo.show_layout(context, layout)
|
PangoCairo.show_layout(context, layout)
|
||||||
|
|
||||||
context.restore()
|
context.restore()
|
||||||
@ -3326,8 +3353,8 @@ class MyTimelineView(NavigationView):
|
|||||||
# Get text size in pixels
|
# Get text size in pixels
|
||||||
text_width, text_height = layout.get_pixel_size()
|
text_width, text_height = layout.get_pixel_size()
|
||||||
|
|
||||||
context.set_source_rgb(0.0, 0.0, 0.0) # Black
|
context.set_source_rgb(*COLOR_BLACK)
|
||||||
context.move_to(timeline_x - 20 - text_width, y_pos - text_height / 2)
|
context.move_to(timeline_x - YEAR_MARKER_LABEL_OFFSET - text_width, y_pos - text_height / 2)
|
||||||
PangoCairo.show_layout(context, layout)
|
PangoCairo.show_layout(context, layout)
|
||||||
|
|
||||||
def draw_year_markers(self, context: cairo.Context, timeline_x: float,
|
def draw_year_markers(self, context: cairo.Context, timeline_x: float,
|
||||||
@ -3406,12 +3433,9 @@ class MyTimelineView(NavigationView):
|
|||||||
vertical_line_x = self._get_person_vertical_line_x(person_index)
|
vertical_line_x = self._get_person_vertical_line_x(person_index)
|
||||||
|
|
||||||
# Find all events for this person
|
# Find all events for this person
|
||||||
person_events = [
|
person_events = self._get_events_for_person(person_handle, events_with_y_pos)
|
||||||
event_data for event_data in events_with_y_pos
|
|
||||||
if self._person_matches_handle(event_data.person, person_handle)
|
|
||||||
]
|
|
||||||
|
|
||||||
if len(person_events) < 1:
|
if not person_events:
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Sort by Y position
|
# Sort by Y position
|
||||||
@ -3422,12 +3446,12 @@ class MyTimelineView(NavigationView):
|
|||||||
|
|
||||||
# Draw vertical connector line on the left side
|
# Draw vertical connector line on the left side
|
||||||
if len(person_events) > 1:
|
if len(person_events) > 1:
|
||||||
y_positions = [event_data.y_pos for event_data in person_events]
|
# Use first and last elements since list is sorted by y_pos
|
||||||
min_y = min(y_positions)
|
min_y = person_events[0].y_pos
|
||||||
max_y = max(y_positions)
|
max_y = person_events[-1].y_pos
|
||||||
|
|
||||||
# Draw vertical line connecting all events
|
# Draw vertical line connecting all events
|
||||||
if max_y - min_y > EVENT_MARKER_SIZE * 2:
|
if max_y - min_y > CONNECTION_LINE_MIN_DISTANCE:
|
||||||
context.move_to(vertical_line_x, min_y)
|
context.move_to(vertical_line_x, min_y)
|
||||||
context.line_to(vertical_line_x, max_y)
|
context.line_to(vertical_line_x, max_y)
|
||||||
context.stroke()
|
context.stroke()
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user