Skip to content

Make 'Open in canvas' on the lead mini-card actually do something#16

Open
jerelvelarde wants to merge 1 commit into
mainfrom
jerel/lead-mini-card-open-in-canvas
Open

Make 'Open in canvas' on the lead mini-card actually do something#16
jerelvelarde wants to merge 1 commit into
mainfrom
jerel/lead-mini-card-open-in-canvas

Conversation

@jerelvelarde

Copy link
Copy Markdown
Owner

Summary

  • Clicking Open in canvas on <LeadMiniCard> was only setting selectedLeadId — a faint lavender ring on the matching kanban card. If the lead was filtered out or scrolled below the fold, the click looked like a no-op.
  • Click handler now also clears the active filter, marks the lead as highlighted (the existing 2s pulse), and PipelineBoard scrolls the DraggableLeadCard into view via a useEffect on selected.

Test plan

  • In chat, ask the agent to mention a specific lead (e.g. "Tell me about Ada Lovelace and show her mini card."). The mini card mounts.
  • Apply a filter that hides Ada (e.g. setFilter({workshops: ["Deploying Agents (prod)"]}) if Ada isn't in that bucket).
  • Click Open in canvas on the mini card. Filter clears, the kanban scrolls so Ada's card is in view, the card pulses for ~2s and keeps the selection ring.

🤖 Generated with Claude Code

Clicking the LeadMiniCard's "Open in canvas" was setting selectedLeadId
on agent state — which only added a faint lavender ring to the matching
kanban card. If the lead was filtered out or scrolled out of view, the
click looked like a no-op.

Now the click also clears any active filter, sets the lead as
highlighted (triggers the existing 2s pulse), and PipelineBoard scrolls
the matching DraggableLeadCard into view via useEffect on `selected`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request implements a feature to automatically scroll lead cards into view when selected, specifically targeting the 'Open in canvas' action. The changes include clearing filters and highlighting the selected lead in the state. Feedback suggests stabilizing the ref merger with useCallback to avoid performance overhead and refining the scrolling logic to use block: 'nearest' for manual board clicks to prevent jarring UI movements. It is also recommended to include highlightedLeadIds in the scroll effect's dependency array to ensure consistent behavior on repeated actions.

"use client";

import { useState } from "react";
import { useEffect, useRef, useState } from "react";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Add useCallback to the React imports to support stabilizing the ref merger and other hooks used within the component.

Suggested change
import { useEffect, useRef, useState } from "react";
import { useCallback, useEffect, useRef, useState } from "react";

Comment on lines +192 to +197
useEffect(() => {
if (!selected) return;
const node = containerRef.current;
if (!node) return;
node.scrollIntoView({ behavior: "smooth", block: "center" });
}, [selected]);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation scrolls the card to the center of the viewport whenever it is selected. While this is effective for the 'Open in canvas' action from chat, it can be jarring for manual clicks on the board where the card is already visible.

Consider using block: "nearest" to avoid unnecessary movement for visible cards, or conditionally use "center" only when the lead is also highlighted (which distinguishes the chat action from a manual board click).

Additionally, adding highlightedLeadIds to the dependency array ensures that clicking 'Open in canvas' multiple times for the same lead still triggers the scroll if the user has manually scrolled away in between (since onSelect in page.tsx creates a new array reference on every click).

  useEffect(() => {
    if (!selected) return;
    const node = containerRef.current;
    if (!node) return;
    // Use 'center' for intentional 'Open in canvas' actions (highlighted),
    // but 'nearest' for manual board selections to avoid jarring jumps.
    node.scrollIntoView({
      behavior: "smooth",
      block: highlighted ? "center" : "nearest",
    });
  }, [selected, highlighted, highlightedLeadIds]);

Comment on lines +200 to +203
ref={(node) => {
setNodeRef(node);
containerRef.current = node;
}}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using an inline function for the ref prop causes React to call the ref callback twice (once with null and once with the element) on every re-render of the component. This can lead to unnecessary overhead and potential issues with dnd-kit's internal tracking. It is better to stabilize the ref merger using useCallback.

Suggested change
ref={(node) => {
setNodeRef(node);
containerRef.current = node;
}}
ref={useCallback((node: HTMLDivElement | null) => {
setNodeRef(node);
containerRef.current = node;
}, [setNodeRef])}

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