feat: floating code popup #39885
feat: floating code popup #39885
Conversation
…with dynamic resizing and styling
alex-golovanov
requested review from
vivek-appsmith
and removed request for
a team
WalkthroughThis change introduces a floating code popup for the property pane. A new dependency ( Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CE as CodeEditor
participant CEC as CodeEditorControl
participant PE as PopoutEditor
U->>CE: Click expand trigger
CE->>CEC: Invoke onExpandTriggerClick
CEC->>CEC: Set popOutVisible=true
CEC->>PE: Render PopoutEditor with props
PE->>PE: Initialize draggable & resizable functionality
PE->>CEC: onClose event when user closes popup
CEC->>CEC: Set popOutVisible=false
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yaml ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (5)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
app/client/src/components/editorComponents/CodeEditor/index.tsx (1)
1732-1745: Good implementation of the expand trigger.The expand trigger is implemented with appropriate conditional rendering and accessibility features:
- Uses a Tooltip for better UX
- Proper button kind and icon choice
- Correctly passes the click handler
One minor suggestion: consider adding an aria-label for better accessibility.
<Button className="commands-button invisible" isIconButton kind="secondary" onClick={this.props.onExpandTriggerClick} size="sm" startIcon="external-link-line" + aria-label="Expand code editor" />app/client/src/components/propertyControls/CodeEditorControl.tsx (1)
46-76: Consider enhancing the placeholder UI.The Flex component rendered when popOutVisible is true serves as a placeholder but doesn't provide users with context about the expanded state. Consider adding text or an icon to indicate that an editor is currently expanded.
<Flex alignItems="center" bg="var(--ads-v2-color-bg)" border="1px solid var(--ads-v2-color-border)" borderRadius="var(--ads-v2-border-radius)" h="36px" justifyContent="center" w="100%" > + <Text kind="body-s">Editor expanded in floating window</Text> </Flex>app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx (3)
77-82: Use destructured value for consistencyIn the x calculation, you're using
defaultWithdirectly from the destructured variables, but in the window width calculation you reference it indirectly. Update for consistency.return { - x: Math.max((window.innerWidth - defaultWith) / 2, 0), + x: Math.max((window.innerWidth - defaultWidth) / 2, 0), y: Math.max((window.innerHeight - defaultHeight) / 2, 0), width: defaultWith, height: defaultHeight, };
93-95: Consider using a more descriptive debounce timeout constant nameThe constant name
RESIZE_DEBOUNCE_TIMEOUTis generic. Consider renaming it to something more specific likeEDITOR_RESIZE_DEBOUNCE_TIMEOUTto clarify its purpose.
44-55: Consider making required props explicitThe
PopoutEditorPropsinterface extendsPartial<EditorProps>which makes all EditorProps optional, but then explicitly requires some props. Consider using a more specific approach.-export interface PopoutEditorProps extends Partial<EditorProps> { +export interface PopoutEditorProps { + // Required props additionalAutocomplete?: AdditionalDynamicDataTree; dataTreePath?: string; expected?: CodeEditorExpected; hideEvaluatedValue?: boolean; label: string; onChange: EventOrValueHandler<ChangeEvent<HTMLTextAreaElement>>; onClose: () => void; theme: EditorTheme; value: string; widgetName: string; + // Optional EditorProps + [key: string]: any; // Or list specific EditorProps you need }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
app/client/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
app/client/package.json(1 hunks)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx(1 hunks)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/constants.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/index.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/styles.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/index.tsx(3 hunks)app/client/src/components/propertyControls/CodeEditorControl.tsx(2 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
app/client/src/components/propertyControls/CodeEditorControl.tsx (2)
app/client/src/components/editorComponents/CodeEditor/index.tsx (1)
EditorProps(227-266)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx (1)
PopoutEditor(57-183)
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx (2)
app/client/src/components/editorComponents/CodeEditor/index.tsx (2)
EditorProps(227-266)CodeEditorExpected(169-174)app/client/src/components/editorComponents/CodeEditor/PopoutEditor/constants.ts (5)
RND_CONFIG(15-21)RESIZE_DEBOUNCE_TIMEOUT(23-23)DRAG_HANDLE_CLASS_NAME(12-12)RESIZE_HANDLE_STYLES(1-10)RESIZE_HANDLE_CLASS_NAME(13-13)
🔇 Additional comments (13)
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/index.ts (1)
1-2: Clean export implementation.This export statement properly exposes the PopoutEditor component from its implementation file, following standard module export patterns.
app/client/package.json (1)
196-196: Appropriate dependency addition.The react-rnd package is a good choice for implementing draggable and resizable UI elements. Version 10.5.2 is stable and compatible with the existing React version.
app/client/src/components/editorComponents/CodeEditor/index.tsx (2)
77-77: Proper import for button components.The necessary UI components for the expand functionality are correctly imported from the design system.
187-190: Well-structured interface extension.The added properties to the EditorStyleProps interface clearly define the contract for the expand functionality with appropriate optional flags.
app/client/src/components/propertyControls/CodeEditorControl.tsx (3)
15-16: Appropriate component imports.The PopoutEditor and Flex components are correctly imported for implementing the floating editor functionality.
19-25: Clean state management implementation.Good implementation of state management for the popup visibility with a clear state setter method.
77-87: Well-implemented PopoutEditor integration.The PopoutEditor is properly implemented with all necessary props passed through. The onClose handler correctly resets the state when the popup is closed.
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/styles.ts (4)
3-14: LGTM: Backdrop implementation with correct pointer eventsThe Backdrop component correctly uses
pointer-events: noneto allow interaction with elements beneath it while providing visual context.
16-29: LGTM: Well-structured PopoutContainer with proper stylingThe PopoutContainer properly sets
pointer-events: autoto re-enable mouse events within the container, with appropriate z-index levels to ensure proper layering.
31-40: LGTM: Header with proper drag functionality settingsThe header component correctly implements styles needed for the drag functionality, including
cursor: moveanduser-select: noneto prevent text selection while dragging.
42-51: LGTM: EditorContainer with appropriate CodeMirror stylingThe EditorContainer properly manages overflow and whitespace styles, with specific styling for CodeMirror to preserve whitespace as needed.
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx (2)
102-121: LGTM: Efficient use of ResizeObserver with proper cleanupGood implementation of ResizeObserver to track container size changes, with proper cleanup in the useLayoutEffect hook.
127-182: LGTM: Well-structured portal implementation with proper component compositionThe portal implementation is well-structured with appropriate component composition. The use of the Rnd component for draggable and resizable functionality is implemented correctly.
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/constants.ts
Outdated
Show resolved
Hide resolved
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx
Outdated
Show resolved
Hide resolved
app/client/src/components/editorComponents/CodeEditor/PopoutEditor/PopoutEditor.tsx
Show resolved
Hide resolved
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14056585913. |
github-actions
bot
added
IDE Navigation
|
/build-deploy-preview skip-tests=true |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/14067684915. |
|
Deploy-Preview-URL: https://ce-39885.dp.appsmith.com |
Description
Pop-up editor for property pane.
Fixes #34541
Automation
/ok-to-test tags="@tag.All"
🔍 Cypress test results
Caution
🔴 🔴 🔴 Some tests have failed.
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/14079811857
Commit: 3f357d3
Cypress dashboard.
Tags: @tag.All
Spec:
The following are new failures, please fix them before merging the PR:
- cypress/e2e/Regression/ClientSide/FormLogin/EnableFormLogin_spec.js
- cypress/e2e/Regression/ClientSide/IDE/Canvas_Context_Property_Pane_2_spec.js
List of identified flaky tests.Wed, 26 Mar 2025 11:16:34 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
Style