adjust padding and height for chat editor and welcome widget#297693
adjust padding and height for chat editor and welcome widget#297693
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements dynamic height adjustment for the chat editor in the Agent Sessions window and adds horizontal padding to the welcome widget. The changes enable the editor to grow and shrink based on content while respecting minimum and maximum height constraints, improving the user experience during text input.
Changes:
- Added dynamic height adjustment to the chat editor that responds to content size changes (50-200px range)
- Updated CSS min-height from 36px to 50px to align with the new minimum height constraint
- Added horizontal padding (12px) to the chat welcome container for better visual spacing
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/vs/sessions/contrib/chat/browser/newChatViewPane.ts | Added _editorContainer field and onDidContentSizeChange handler to dynamically adjust editor height based on content |
| src/vs/sessions/contrib/chat/browser/media/chatWidget.css | Updated min-height from 36px to 50px to match the minimum height constraint used in TypeScript |
| src/vs/sessions/contrib/chat/browser/media/chatWelcomePart.css | Added horizontal padding (0 12px 10% 12px) to the welcome container for consistent spacing |
| this._register(this._editor.onDidContentSizeChange(() => { | ||
| const contentHeight = this._editor.getContentHeight(); | ||
| const clampedHeight = Math.min(200, Math.max(50, contentHeight)); | ||
| this._editorContainer.style.height = `${clampedHeight}px`; | ||
| this._editor.layout(); |
There was a problem hiding this comment.
The implementation should check if the height actually changed before updating the DOM and calling layout(). Similar implementations in chatInputPart.ts (line 2114) check e.contentHeightChanged before proceeding. Additionally, consider tracking the previous height to avoid unnecessary DOM updates when the clamped height remains the same, as this event can fire frequently during typing.
|
|
||
| this._register(this._editor.onDidContentSizeChange(() => { | ||
| const contentHeight = this._editor.getContentHeight(); | ||
| const clampedHeight = Math.min(200, Math.max(50, contentHeight)); |
There was a problem hiding this comment.
The CSS defines min-height: 50px and max-height: 200px, but the TypeScript clamping uses Math.min(200, Math.max(50, contentHeight)). While functionally equivalent, this creates a potential maintenance issue where the height constraints are duplicated in two places. If these values need to change in the future, both locations must be updated. Consider extracting these as named constants (e.g., MIN_EDITOR_HEIGHT, MAX_EDITOR_HEIGHT) that can be shared or at least documented together.
| @@ -41,7 +41,7 @@ | |||
| .sessions-chat-editor { | |||
| padding: 0 6px 6px 6px; | |||
| height: 50px; | |||
There was a problem hiding this comment.
The CSS static height property (line 43: height: 50px) may conflict with the dynamic height being set via inline styles in TypeScript (line 442). When both are present, the inline style will take precedence, but this creates confusion about the source of truth for the height. Consider removing the static height: 50px from the CSS since it's now being managed dynamically, leaving only min-height and max-height as constraints.
| height: 50px; |
No description provided.