Skip to content

Conversation

@iceljc
Copy link
Collaborator

@iceljc iceljc commented Sep 11, 2025

PR Type

Enhancement


Description

  • Add smooth scrolling to agent component containers

  • Auto-scroll to bottom when adding new items

  • Improve user experience in agent configuration forms


Diagram Walkthrough

flowchart LR
  A["User clicks Add button"] --> B["New item added to list"]
  B --> C["scrollToBottom() function called"]
  C --> D["Container scrolls smoothly to bottom"]
Loading

File Walkthrough

Relevant files
Enhancement
agent-knowledge-base.svelte
Add scrolling to knowledge base component                               

src/routes/page/agent/[agentId]/agent-components/agent-knowledge-base.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new knowledge base
+15/-1   
agent-mcp-tool.svelte
Add scrolling to MCP tool component                                           

src/routes/page/agent/[agentId]/agent-components/agent-mcp-tool.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new MCP tool
+16/-1   
agent-rule.svelte
Add scrolling to agent rule component                                       

src/routes/page/agent/[agentId]/agent-components/agent-rule.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new rule
+15/-1   
agent-utility.svelte
Add scrolling to agent utility component                                 

src/routes/page/agent/[agentId]/agent-components/agent-utility.svelte

  • Add scrollContainer variable for DOM reference
  • Implement scrollToBottom() function with smooth scrolling
  • Bind scroll container to agent utility container div
  • Call scrollToBottom() when adding new utility
+17/-1   

@iceljc iceljc merged commit 36e078b into SciSharp:main Sep 11, 2025
1 of 2 checks passed
@qodo-merge-pro
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

DOM Flush Timing

Ensure the scroll runs after the DOM has actually rendered the newly added item. Consider using Svelte's tick() (or afterUpdate) instead of setTimeout to guarantee timing and avoid potential race conditions.

function scrollToBottom() {
    if (scrollContainer) {
        setTimeout(() => {
            scrollContainer.scrollTo({
                top: scrollContainer.scrollHeight,
                behavior: 'smooth'
            });
        }, 0);
    }
}
Scroll Trigger Placement

The scroll is triggered immediately after updating the list; if subsequent updates occur in handleAgentChange(), the final height may change after the scheduled scroll. Consider awaiting a DOM flush before scrolling, or triggering scroll in a reactive block based on list length.

scrollToBottom();
handleAgentChange();
Duplicate Logic

The same scrollToBottom() implementation appears in multiple components. Consider extracting a shared utility or a Svelte action to reduce duplication and keep behavior consistent.

function scrollToBottom() {
    if (scrollContainer) {
        setTimeout(() => {
            scrollContainer.scrollTo({
                top: scrollContainer.scrollHeight,
                behavior: 'smooth'
            });
        }, 0);
    }
}

@qodo-merge-pro
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Safely schedule and re-check scroll

Re-check the container inside the async callback and use requestAnimationFrame
to run after the next paint. This avoids errors if the component unmounts before
the timeout fires and ensures the DOM has rendered before scrolling.

src/routes/page/agent/[agentId]/agent-components/agent-utility.svelte [276-285]

 function scrollToBottom() {
-    if (scrollContainer) {
-        setTimeout(() => {
-            scrollContainer.scrollTo({
-                top: scrollContainer.scrollHeight,
-                behavior: 'smooth'
-            });
-        }, 0);
-    }
+    // Run after the next paint to ensure DOM/layout is ready
+    requestAnimationFrame(() => {
+        if (!scrollContainer) return;
+        scrollContainer.scrollTo({
+            top: scrollContainer.scrollHeight,
+            behavior: 'smooth'
+        });
+    });
 }
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly proposes using requestAnimationFrame which is better suited for DOM manipulations like scrolling, and it adds a safety check inside the callback, making the code more robust against edge cases.

Low
  • More

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant