[fix] Forward className in Editor noProvider mode and remove CSS workarounds#4555
[fix] Forward className in Editor noProvider mode and remove CSS workarounds#4555mmabrouk wants to merge 2 commits into
Conversation
dosubot
Bot
added
the
size:M
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Railway Preview Environment
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-playground-ui/src/components/ExecutionItemComparisonView/GenerationComparisonChatOutput/index.tsx (1)
203-210: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winInconsistent styling approach: container hack still present.
The
ChatTurnViewat line 205 still uses the container selector hack[&_.agenta-editor-wrapper]:!p-3alongsideeditorClassName: "!p-3"at line 206. This is inconsistent with theTurnMessageAdapterchange above (lines 153-158), which removed the container hack and uses onlyeditorClassName.For consistency and to avoid potential double-padding or override conflicts, consider applying the same refactor here: remove
[&_.agenta-editor-wrapper]:!p-3from the className and rely solely oneditorClassName: "!p-3".♻️ Proposed fix to remove the container hack
className: - "!p-0 [&_.agenta-editor-wrapper]:!p-3 !mt-0 [&:nth-child(1)]:!mt-0 mt-2", + "!p-0 !mt-0 [&:nth-child(1)]:!mt-0 mt-2", editorClassName: "!p-3",
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fb310ad8-c181-48e1-9351-54e0be6b445b
📒 Files selected for processing (5)
web/oss/src/styles/globals.cssweb/packages/agenta-playground-ui/src/components/ExecutionItemComparisonView/GenerationComparisonChatOutput/index.tsxweb/packages/agenta-playground-ui/src/components/adapters/VariableControlAdapter.tsxweb/packages/agenta-ui/src/ChatMessage/components/ChatMessageEditor.tsxweb/packages/agenta-ui/src/Editor/Editor.tsx
…up workarounds The shared Editor dropped `className`/`editorClassName` when rendered with `noProvider` (it only applied the class on the EditorProvider path). Forward it to EditorInner and apply it to the editor container in that mode too. With the prop working, clean up the call sites that relied on the drop: - ChatMessageEditor: move the message text/placeholder alignment from the globals.css workaround to editorClassName; globals.css now only carries the antd role-button override that cannot go through the prop. - VariableControlAdapter: stop forwarding the generation-row container class as editorClassName (it is a container/cell-strip style that would strip the editor's border and gutter); it stays on the container only. - GenerationComparisonChatOutput: pad the editor body via editorClassName instead of the [&_.agenta-editor-wrapper] container hack.
…l padding The comparison view renders messages with their own editor padding (!p-3 cells). The role-alignment inset added by ChatMessageEditor stacked on top of that, over-padding the message text, and the assistant output cell also double-padded (the [&_.agenta-editor-wrapper]:!p-3 hack plus editorClassName). Add an alignTextWithRole prop (default true) to ChatMessageEditor; the comparison view passes false so the inset and role pin are skipped there. Remove the leftover wrapper-padding hack so the comparison cells pad the editor once, via editorClassName.
mmabrouk
force-pushed
the
fix/editor-noprovider-classname
branch
from
2bc3876 to
27851d3
Compare
1 participant
Stacked on #4554.
Context
The shared
Editor(@agenta/ui) silently droppedclassName/editorClassNamewhenever it was rendered withnoProvider. The provider path applied the class to.agenta-rich-text-editor, but thenoProviderpath renderedEditorInnerdirectly and never forwarded the class, andEditorInnerdidn't apply one either. SinceChatMessageEditorand several other editors usenoProvider, any styling passed througheditorClassNamewas a no-op. That is why the message-text alignment in #4554 had to be done with a global CSS workaround.Changes
Editornow passesclassNametoEditorInnerinnoProvidermode, andEditorInnerapplies it to its container div. Provider mode is unchanged (it still lands on.agenta-rich-text-editor, andEditorInnergets noclassNamethere, so there's no double application).With the prop working, the call sites that worked around the drop are cleaned up:
editorClassName([&_.editor-input:not(.code-only)]:px-2,[&_.editor-placeholder]:left-2).globals.csskeeps only the antd role-button override, which can't go through the editor prop.editorClassName={className}in the rich-text branch. That class is a container/cell-strip style (*:!border-none px-3); the file already documents it must stay off the editor (it strips the editor's border and gutter). It remains applied to the container only.editorClassName: "!p-3"instead of the[&_.agenta-editor-wrapper]:!p-3container hack (which would otherwise double-pad now that the prop works).Behavior change to be aware of
Two preview surfaces passed
editorClassNamefont-size classes that were dead until now and will start applying (this is the intended styling, previously broken by the bug):renderChatMessages):!text-xsPromptPreview):!text-smTests
tsc --noEmitpasses for@agenta/uiand@agenta/playground-ui.