fix: improve layout and scrolling in settings panels #90

ErikBjare · 2025-09-26T13:45:05Z

Fix flex layout and padding in ConversationSettings
Remove problematic overflow-auto from RightSidebarContent
Simplify tools configuration description
Clean up conversation loading logic in useConversation hook

Important

Improve layout and scrolling in settings panels, refine conversation loading logic, and enhance session handling.

Layout Improvements:
- Adjust flex layout and padding in ConversationSettings.
- Remove overflow-auto from RightSidebarContent.
- Simplify description in ToolsConfiguration.
Logic Enhancements:
- Refine conversation loading logic in useConversation to handle existing messages and connection states more efficiently.
- Ensure session reuse and connection handling in ApiClient.
Miscellaneous:
- Adjust MainLayout to handle conversation and task selection more robustly.

^{This description was created by}^{for bc6e093. You can customize this summary. It will automatically update as commits are pushed.}

- Fix flex layout and padding in ConversationSettings - Remove problematic overflow-auto from RightSidebarContent - Simplify tools configuration description - Clean up conversation loading logic in useConversation hook

ellipsis-dev

Important

Looks good to me! 👍

Reviewed everything up to dce0fae in 47 seconds. Click for details.

Reviewed 70 lines of code in 4 files
Skipped 0 files when reviewing.
Skipped posting 6 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/components/ConversationSettings.tsx:50

Draft comment:
Adding h-full to the container ensures the panel fills its parent’s height. Verify that the parent element has an explicit height set.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

2. src/components/ConversationSettings.tsx:88

Draft comment:
Bottom padding reduced from 'pb-24' to 'pb-12'. Confirm this spacing adjustment matches the design specs.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

3. src/components/ConversationSettings.tsx:229

Draft comment:
Replaced sticky footer classes with 'flex-shrink-0'. Ensure that the submit button scrolling behavior is as intended since it will no longer remain fixed.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

4. src/components/RightSidebarContent.tsx:36

Draft comment:
Removed the 'overflow-auto' class from the container. Check that inner components handle scrolling to prevent content overflow issues.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

5. src/components/settings/ToolsConfiguration.tsx:54

Draft comment:
Simplified the tools configuration description text. This improves clarity; ensure the shortened text still meets user needs.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

6. src/hooks/useConversation.ts:68

Draft comment:
Refactored the check for existing messages using conversation$?.data.log.get()?.length. Confirm that log.get() reliably returns an array and that this change aligns with reactive state management.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_tjfgBxZP7MPaxbh9

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

…l prompts Previously, the SSE connection would create a new session instead of reusing the session from createConversation(), causing interrupts to fail with 404 because generation was on a different session than the interrupt request. Changes: - Pass existing session_id to SSE endpoint to reuse the same session - Prevent createConversation() from overwriting SSE-established sessions - Add onConnected callback to track when SSE connection is established Fixes interrupt functionality on initial conversation prompts and resolves related 404 errors for tool confirmations. Related: #92

- Implement onConnected callback to properly track SSE connection state - Fix setConnected being called immediately instead of on SSE connect - Change .get() to .peek() for performance (avoid unnecessary reactivity) - Add debug logging to help troubleshoot streaming and connection issues - Improve placeholder message handling logic Related to the interrupt fix - ensures connection state is properly synchronized with SSE events. Related: #92

ellipsis-dev

Caution

Changes requested ❌

Reviewed dd8ec0f in 1 minute and 51 seconds. Click for details.

Reviewed 98 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 2 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/hooks/useConversation.ts:68

Draft comment:
Using .peek() instead of .get() bypasses reactivity. Confirm that this change is intentional, as it may affect state updates.
Reason this comment was not posted:
Comment looked like it was already resolved.

2. src/hooks/useConversation.ts:303

Draft comment:
Moving setConnected(...) inside the onConnected callback is cleaner. Ensure that the API reliably triggers onConnected, as this change removes the immediate connection update.
Reason this comment was not posted:
Comment looked like it was already resolved.

Workflow ID: wflow_oxprPyVYbKtUruoW

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

ellipsis-dev · 2025-09-30T07:52:53Z

src/hooks/useConversation.ts

-          existingConversation && existingConversation.data.log.length > 0;
-
+        const hasExistingMessages = conversation$?.data.log.peek()?.length > 0;
+        console.log('[useConversation] Loading conversation', {


The newly added console.log statements (e.g., logging conversation load info) may be too verbose for production. Consider gating these behind a debug flag or using a proper logging utility.

src/hooks/useConversation.ts

Add guards to only update selectedConversation$ and selectedTask$ when the value actually changes, preventing unnecessary re-renders and potential infinite loops.

ellipsis-dev

Important

Looks good to me! 👍

Reviewed d2f4198 in 41 seconds. Click for details.

Reviewed 29 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/components/MainLayout.tsx:89

Draft comment:
Good update – checking if the current state already matches the new value avoids unnecessary updates and re-renders. The conditional clearing (only resetting if not already empty) adds extra safety. Consider adding a brief comment that conversationId takes precedence over taskId when both are provided.
Reason this comment was not posted:
Confidence changes required: 33% <= threshold 50% None

Workflow ID: wflow_Nvg7WROsGUVMGiPb

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

Removed verbose console.log statements that were printing on every token received during streaming. Kept only the important warning for when there's no assistant message to append to.

ellipsis-dev

Important

Looks good to me! 👍

Reviewed bc6e093 in 34 seconds. Click for details.

Reviewed 26 lines of code in 1 files
Skipped 0 files when reviewing.
Skipped posting 1 draft comments. View those below.
Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. src/hooks/useConversation.ts:160

Draft comment:
Removed excessive console logs in the onToken callback for cleaner output. If you need debug logging in development, consider gating logs behind a debug flag or using a logging library.
Reason this comment was not posted:
Confidence changes required: 0% <= threshold 50% None

Workflow ID: wflow_eeicHALXG3irrXkk

^{You can customize}^{by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

fix: improve layout and scrolling in settings panels

dce0fae

- Fix flex layout and padding in ConversationSettings - Remove problematic overflow-auto from RightSidebarContent - Simplify tools configuration description - Clean up conversation loading logic in useConversation hook

ellipsis-dev bot reviewed Sep 26, 2025

View reviewed changes

ErikBjare added 2 commits September 30, 2025 09:49

ellipsis-dev bot reviewed Sep 30, 2025

View reviewed changes

perf: prevent unnecessary state updates in MainLayout

d2f4198

Add guards to only update selectedConversation$ and selectedTask$ when the value actually changes, preventing unnecessary re-renders and potential infinite loops.

ellipsis-dev bot reviewed Sep 30, 2025

View reviewed changes

refactor: remove spammy per-token debug logging

bc6e093

Removed verbose console.log statements that were printing on every token received during streaming. Kept only the important warning for when there's no assistant message to append to.

ellipsis-dev bot reviewed Sep 30, 2025

View reviewed changes

ErikBjare merged commit 9dadf72 into master Sep 30, 2025
1 of 2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: improve layout and scrolling in settings panels #90

fix: improve layout and scrolling in settings panels #90

Uh oh!

ErikBjare commented Sep 26, 2025 •

edited by ellipsis-dev bot

Loading

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot Sep 30, 2025

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

ellipsis-dev bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

fix: improve layout and scrolling in settings panels #90

fix: improve layout and scrolling in settings panels #90

Uh oh!

Conversation

ErikBjare commented Sep 26, 2025 • edited by ellipsis-dev bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

ellipsis-dev bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ErikBjare commented Sep 26, 2025 •

edited by ellipsis-dev bot

Loading