Skip to content

Comments

chore(chat): consolidate chat message whitespacing style#8696

Merged
jmelahman merged 6 commits intomainfrom
jamison/message-gap
Feb 24, 2026
Merged

chore(chat): consolidate chat message whitespacing style#8696
jmelahman merged 6 commits intomainfrom
jamison/message-gap

Conversation

@jmelahman
Copy link
Contributor

@jmelahman jmelahman commented Feb 23, 2026

Description

Standardizes the spaces between chat messages by applying gap-12 on the message container rather than rely on individual messages aligning themselves with padding.

Closes https://linear.app/onyx-app/issue/ENG-3744/the-user-input-is-misaligned

How Has This Been Tested?

Reviewed the visual regression report and all changes look desired


Summary by cubic

Standardized chat message spacing by moving vertical gaps and right padding to the ChatUI container, and made HumanMessage action buttons responsive to prevent wrap-induced layout shifts. Fixes misaligned user input and keeps layouts consistent across the chat (Linear ENG-3744).

  • Refactors
    • Applied flex flex-col with gap-12, pt-4, pb-8, pr-1 on the ChatUI container; removed the top Spacer and per-message padding; DynamicBottomSpacer now has w-full to satisfy Playwright.
    • HumanMessage: removed pt/pb and -mr-6; message row is md:flex and relative; action buttons are absolute on small screens and relative on md+ with z-content, right-anchored. AgentMessage: removed padding (including pr-1) and relies on container gap.

Written for commit 3a397cf. Summary will update on new commits.

@jmelahman jmelahman requested a review from a team as a code owner February 23, 2026 23:06
@github-actions
Copy link
Contributor

github-actions bot commented Feb 23, 2026

Preview Deployment

Status Preview Commit Updated
https://onyx-preview-3hncvfl66-danswer.vercel.app 3a397cf 2026-02-24 19:42:44 UTC

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 23, 2026

Greptile Summary

This PR consolidates chat message spacing by moving whitespace control from individual message components to the ChatUI container. The container now uses gap-12 to standardize spacing between messages, replacing the previous approach where each message applied its own padding (pt-5, pb-1, pb-5, md:pt-5). Additionally, the negative margin hack (-mr-6) was removed from HumanMessage, and the action buttons (edit/copy) were made responsive—positioned absolutely on mobile and relatively on desktop to prevent layout shifts when they appear.

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are purely presentational CSS refactoring that centralizes spacing control. The visual regression report was reviewed and approved. No logic changes, breaking changes, or security concerns.
  • No files require special attention

Important Files Changed

Filename Overview
web/src/sections/chat/ChatUI.tsx Moved spacing from individual messages to container using flex gap-12, pt-4, pb-8, pr-1; removed top Spacer; wrapped return in fragment
web/src/app/app/message/HumanMessage.tsx Removed pt-5, pb-1, -mr-6; made message row md:flex with relative positioning; positioned action buttons absolutely on mobile, relatively on desktop
web/src/app/app/message/messageComponents/AgentMessage.tsx Removed pb-5, md:pt-5, pr-1 padding to rely on container gap-12 spacing

Last reviewed commit: 15015a4

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 23, 2026

Additional Comments (1)

web/src/sections/chat/ChatUI.tsx
Unused import - Spacer is no longer used after removing <Spacer /> from line 117

import DynamicBottomSpacer from "@/components/chat/DynamicBottomSpacer";

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

@github-actions
Copy link
Contributor

github-actions bot commented Feb 23, 2026

🖼️ Visual Regression Report

Project Changed Added Removed Unchanged Report
admin 38 0 0 71 View Report
exclusive 0 0 0 8 ✅ No changes

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

6 issues found across 90 files (changes from recent commits).

Note: This PR contains a large number of files. cubic only reviews up to 75 files per PR, so some files may not have been reviewed.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="backend/onyx/tools/tool_implementations/python/python_tool.py">

<violation number="1">
P2: Raising on StreamErrorEvent skips the cleanup of staged files/generate files, which can leak Code Interpreter files. Handle the error without bypassing cleanup (e.g., store the error and run cleanup in a finally block).</violation>
</file>

<file name="web/src/sections/modals/PreviewModal/variants/csvVariant.tsx">

<violation number="1">
P2: CSV parsing uses simple string split, which breaks on quoted commas and other valid CSV cases. Use a real CSV parser (or a parser that handles quoted fields) to avoid incorrect column counts and misrendered data.</violation>
</file>

<file name="backend/requirements/model_server.txt">

<violation number="1">
P2: This requirements .txt file is documented as generated; updates should be made in pyproject.toml and regenerated to avoid drift. Please update the source dependency spec and re-export requirements instead of editing this file directly.</violation>
</file>

<file name="web/src/sections/modals/PreviewModal/variants/shared.tsx">

<violation number="1">
P2: Avoid nesting a Button inside an <a>; it creates invalid interactive markup and can interfere with activation/accessibility. Render a single element by using the Button’s link props (href/download) instead of wrapping it in an anchor.</violation>
</file>

<file name="extensions/chrome/src/pages/panel.js">

<violation number="1">
P2: Unhandled throw from `getIframeOrigin()` — if a `TAB_URL_UPDATED` message arrives before `setIframeSrc()` is called (initialization is async), `new URL(iframe.src)` will throw an uncaught exception. The `handleMessage` function correctly wraps this call in try-catch; this handler should do the same to fail closed.</violation>
</file>

<file name="backend/onyx/tools/tool_implementations/python/code_interpreter_client.py">

<violation number="1">
P1: Streaming response resource leak: the `stream=True` response is not closed on the non-404 path if `raise_for_status()` throws, `_parse_sse` raises (e.g., malformed JSON), or the caller abandons the generator. Wrap in `try/finally` to ensure `response.close()` is always called.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

@jmelahman jmelahman enabled auto-merge February 24, 2026 18:52
@jmelahman
Copy link
Contributor Author

@greptile plrease re-review

@jmelahman jmelahman added this pull request to the merge queue Feb 24, 2026
Merged via the queue into main with commit 255ba10 Feb 24, 2026
86 checks passed
@jmelahman jmelahman deleted the jamison/message-gap branch February 24, 2026 20:09
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.

2 participants