PR #112 — feat/timeline-component Merged 6 comments

  1. Will Riley Yesterday at 9:14 am

    Opening for review. Added the TimelineComponent with vertical, horizontal, compact, and alternating variants. Tests cover 216 assertions across all four gemfile targets. Previews are in elements/timeline_component_preview.

    Reply

    1. Amy Kim 9:41 am

      Looks great overall. One question — the nested thread connector uses a hardcoded -2.075rem left offset. Is that derived from a token or is it magic?

    2. Jake Sullivan 10:03 am

      It's connector-col (1.75rem) + col-gap (0.75rem) - line-width (2px) — I'd love to see it expressed as a CSS calc so it self-documents. Otherwise LGTM.

    3. Will Riley 10:22 am

      Good catch — I'll derive it from the tokens in the next commit so it stays in sync if connector-col or col-gap ever change.

  2. Jake Sullivan Yesterday at 10:48 am

    The alternating layout is a nice touch. Did you consider adding a token for the minimum column width so callers can tighten it on narrow viewports?

    Reply

    1. Will Riley 11:05 am

      Good idea — added --timeline-alt-min-col: 12rem and wired it to the 1fr columns. Pushed to the branch.

    2. Jake Sullivan 11:19 am

      Perfect. Approved.

  3. Priya Sharma Today at 8:55 am

    Nit: compact variant last-item feels a bit tight — the bottom of the final dot almost clips the element below it. Could we bump the padding-bottom by 0.25rem?

    Reply

    1. Will Riley 9:08 am

      Fixed in 3a91c2f — overrode the last-child rule in .is-compact to add the extra breathing room.

  4. Amy Kim Today at 9:31 am

    All threads resolved. Merging — nice work everyone.

    Reply