Skip to content

define Geom class hierarchy skeleton (no behavior change)#321

Draft
AviraL0013 wants to merge 1 commit intoanimint:masterfrom
AviraL0013:geom-class-hierarchy
Draft

define Geom class hierarchy skeleton (no behavior change)#321
AviraL0013 wants to merge 1 commit intoanimint:masterfrom
AviraL0013:geom-class-hierarchy

Conversation

@AviraL0013
Copy link
Copy Markdown

Overview

This PR initiates the refactoring of the monolithic draw_geom() function in inst/htmljs/animint.js into a structured, object-oriented Geom class hierarchy.

This work builds upon the foundation started by @Faye-yufan in PR #107, I am restoring those architectural decisions and extending them to cover missing geoms (GeomTallrect, GeomWiderect, GeomRect).

Architectural Changes

I have introduced a base Geom class and two virtual classes to handle the core data-binding distinction in animint2:

  1. Geom (Base): Handles shared logic for panel identification, SVG selection, and scale lookup.
  2. GroupGeom (Virtual): Designed for geoms like path, line, and polygon (data_is_object: true). These geoms require the "kv-based" data-bind trick to bind one SVG element to a group of data rows.
  3. UngroupGeom (Virtual): Designed for standard geoms like point and segment. These bind data row-by-row (one SVG element per data point).

Concrete Subclasses Implemented (Skeleton):

  • GeomSegment, GeomLinerange, GeomVline, GeomHline, GeomText, GeomPoint (Restored from Faye's design)
  • GeomTallrect, GeomWiderect, GeomRect (New additions)

Purpose and Future Work

This refactor is a critical architectural prerequisite for:

@AviraL0013 AviraL0013 marked this pull request as draft April 10, 2026 07:12
@AviraL0013
Copy link
Copy Markdown
Author

Hi @Faye-yufan @tdhock! Started fresh from master as suggested. Pushed the initial skeleton with Geom -> GroupGeom/UngroupGeom split and concrete subclasses for all ungrouped geoms (GeomTallrect, GeomWiderect, GeomRect included). Will migrate geoms one-by-one let me know if the direction looks right before I go further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant