chore: Function Render of State Inspector #38893
Conversation
WalkthroughThe pull request introduces a comprehensive refactoring of the JSON viewer and peek overlay components in the Appsmith client application. The changes focus on improving the rendering and styling of JSON data, introducing a new Changes
Assessment against linked issues
Suggested Labels
Suggested Reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
|
/build-deploy-preview |
github-actions
bot
added
the
skip-changelog
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13028934930. |
|
Deploy-Preview-URL: https://ce-38893.dp.appsmith.com |
|
/build-deploy-preview |
|
Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13049451730. |
|
Deploy-Preview-URL: https://ce-38893.dp.appsmith.com |
github-actions
bot
added
Bug
hetunandu
added
ok-to-test
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (14)
app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/utils.ts (2)
3-5: Add null check for propertyPath parameter.While the function works, it could be more robust with parameter validation.
export const getPropertyData = (src: unknown, propertyPath: string[]) => { + if (!Array.isArray(propertyPath)) return src; return propertyPath.length > 0 ? get(src, propertyPath) : src; };
6-16: Consider using type predicates for better type narrowing.The type checks are correct, but we can improve type safety.
-export const getDataTypeHeader = (data: unknown) => { +export const getDataTypeHeader = (data: unknown): string => { const dataType = typeof data; if (dataType === "object") { if (Array.isArray(data)) return "array"; if (data === null) return "null"; } return dataType; };app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/styles.ts (3)
4-18: Consider using design system variable for box-shadow color.The implementation looks good, but there's a hardcoded color value in the box-shadow property.
Consider replacing the hardcoded color with a design system variable:
- box-shadow: 0 0 10px #0000001a; // color used from designs + box-shadow: 0 0 10px var(--ads-v2-color-shadow);
20-26: LGTM! Consider using design system variable for font size.The styling is clean and consistent with the design system. Consider using a design system variable for the font size if available.
- font-size: 10px; + font-size: var(--ads-v2-font-size-xs);
32-37: Maintain consistency with font size variables.Similar to the DataType component, consider using a design system variable for the font size.
- font-size: 10px; + font-size: var(--ads-v2-font-size-xs);app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/PeekOverlayPopup.tsx (5)
1-18: Fix inconsistent import statement formatting.Line 8 is missing the curly braces for the import statement, unlike other similar imports.
-import { componentWillAppendToBody } from "react-append-to-body"; +import { componentWillAppendToBody } from "react-append-to-body";
57-67: Add type annotations to improve type safety.The destructured array from useMemo lacks type annotations, which could lead to type inference issues.
- const [jsData, dataType] = useMemo( + const [jsData, dataType]: [unknown, string] = useMemo(
71-85: Extract magic numbers into named constants.The value 300 (pixel width) and 8 (minimum left padding) should be extracted into named constants for better maintainability.
+const OVERLAY_WIDTH_PX = 300; +const MIN_LEFT_PADDING_PX = 8; const getPositionValues = useCallback(() => { const positionValues: { $left: string; $bottom?: string; $top?: string } = { - $left: Math.max(position.right - 300, 8) + "px", + $left: Math.max(position.right - OVERLAY_WIDTH_PX, MIN_LEFT_PADDING_PX) + "px", };
69-69: Optimize debounced event handler.Consider using useMemo for the debounced function to prevent recreation on every render.
-const debouncedHide = debounce(hidePeekOverlay, PEEK_OVERLAY_DELAY); +const debouncedHide = useMemo( + () => debounce(hidePeekOverlay, PEEK_OVERLAY_DELAY), + [hidePeekOverlay] +);Also applies to: 87-90, 96-98
Line range hint
111-126: Simplify repetitive rendering logic.The multiple conditional renders for primitive types can be simplified using a map or switch statement.
- {dataType === "function" && <div>{jsData.toString()}</div>} - {dataType === "boolean" && <div>{jsData.toString()}</div>} - {dataType === "string" && <div>{jsData.toString()}</div>} - {dataType === "number" && <div>{jsData.toString()}</div>} + {["function", "boolean", "string", "number"].includes(dataType) && ( + <div>{jsData.toString()}</div> + )}app/client/src/components/editorComponents/JSONViewer/constants.ts (1)
13-22: Add documentation for reactJsonProps configuration.While the configuration is comprehensive, adding JSDoc comments would help explain the purpose and impact of each property.
+/** + * Default configuration for react-json-view component + * @property {boolean} enableClipboard - Enables/disables copy to clipboard + * @property {boolean} displayDataTypes - Shows/hides data type labels + * @property {number} collapsed - Depth level for collapsing nested objects + */ export const reactJsonProps = { name: null, enableClipboard: false, displayDataTypes: false,app/client/src/components/editorComponents/JSONViewer/JSONViewer.tsx (1)
7-16: Consider memoizing the component for better performance.Since the component receives props that could be complex objects, memoization would help prevent unnecessary re-renders.
-export function JSONViewer(props: JSONViewerProps) { +export const JSONViewer = React.memo(function JSONViewer(props: JSONViewerProps) { const fontSize = FontSize[props.size]; const iconSize = IconSize[props.size]; return ( <Styled.Container $fontSize={fontSize} $iconSize={iconSize}> <ReactJson src={props.src} {...reactJsonProps} /> </Styled.Container> ); -} +});app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
73-74: Consider using a more semantic container for overflow content.The Flex component is being used primarily for overflow behavior. A more semantic choice would be a dedicated scrollable container.
-<Flex className="as-mask" overflowY="auto" px="spaces-3"> +<Styled.ScrollContainer className="as-mask" px="spaces-3"> <JSONViewer size={Size.MEDIUM} src={selectedItemCode} /> -</Flex> +</Styled.ScrollContainer>app/client/src/components/editorComponents/JSONViewer/styles.ts (1)
Line range hint
3-105: Consider using CSS custom properties for better maintainability.The CSS template uses repeated font size and icon size props. Consider using CSS custom properties to reduce repetition and improve maintainability.
const ReactJSONViewerOverrider = css<{ $fontSize: string; $iconSize: string }>` + --json-viewer-font-size: ${({ $fontSize }) => $fontSize}; + --json-viewer-icon-size: ${({ $iconSize }) => $iconSize}; font-size: var(--json-viewer-font-size) !important; .node-ellipsis, .function-collapsed span:nth-child(2), .string-value span { - font-size: ${({ $fontSize }) => $fontSize} !important; + font-size: var(--json-viewer-font-size) !important; } // ... rest of the styles .icon-container { - width: ${({ $iconSize }) => $iconSize} !important; - height: ${({ $iconSize }) => $iconSize} !important; + width: var(--json-viewer-icon-size) !important; + height: var(--json-viewer-icon-size) !important; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/Analytics.ts(0 hunks)app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/PeekOverlayPopup.tsx(3 hunks)app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/constants.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/styles.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/utils.ts(1 hunks)app/client/src/components/editorComponents/CodeEditor/index.tsx(3 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.test.tsx(8 hunks)app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx(3 hunks)app/client/src/components/editorComponents/JSONViewer/JSONViewer.tsx(1 hunks)app/client/src/components/editorComponents/JSONViewer/constants.ts(1 hunks)app/client/src/components/editorComponents/JSONViewer/index.ts(1 hunks)app/client/src/components/editorComponents/JSONViewer/styles.ts(3 hunks)app/client/src/components/editorComponents/JSONViewer/types.ts(1 hunks)
💤 Files with no reviewable changes (1)
- app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/Analytics.ts
✅ Files skipped from review due to trivial changes (2)
- app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/constants.ts
- app/client/src/components/editorComponents/JSONViewer/index.ts
🔇 Additional comments (10)
app/client/src/components/editorComponents/CodeEditor/index.tsx (2)
131-131: LGTM! Good refactoring of constant import.Moving the
PEEK_OVERLAY_DELAYconstant to a dedicated constants file improves code organization.Also applies to: 164-164
Line range hint
203-219: Well-structured interface definition for gutter configuration.The
GutterConfiginterface is well-designed with clear property definitions:
line: Line number for the gutterelement: HTML Element to be addedisFocusedAction: Callback for focus eventsThe integration with
EditorPropsthroughcustomGutterproperty is clean.app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/utils.ts (1)
1-1: LGTM!Good choice using Lodash's
getfor safe property access.app/client/src/components/editorComponents/CodeEditor/PeekOverlayPopup/styles.ts (3)
1-2: LGTM! Clean imports.The imports are minimal and appropriate for the styled components implementation.
28-30: LGTM! Simple and effective divider extension.Clean extension of the design system's Divider component.
39-43: LGTM! Well-structured JSON wrapper.Good implementation with appropriate height constraints and scroll behavior.
app/client/src/components/editorComponents/JSONViewer/types.ts (1)
1-9: LGTM! Well-structured type definitions.The type definitions are clear and follow TypeScript best practices. Using
unknownforsrcis a good choice for type safety.app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.tsx (1)
15-18: LGTM! Good extraction of padding constant.Moving padding to a constant improves maintainability and reusability.
app/client/src/components/editorComponents/JSONViewer/styles.ts (1)
107-109: LGTM! Clean styled-component implementation.The Container component correctly implements the CSS template with required props.
app/client/src/components/editorComponents/Debugger/StateInspector/StateInspector.test.tsx (1)
36-40: LGTM! Consistent theme provider implementation.All test cases are properly wrapped with ThemeProvider, ensuring consistent theme context.
Also applies to: 62-66, 84-88, 101-105, 122-126, 131-135, 145-149, 163-167
Description
Fixes #38874
Automation
/ok-to-test tags="@tag.IDE"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13066102138
Commit: b7319e7
Cypress dashboard.
Tags:
@tag.IDESpec:
Fri, 31 Jan 2025 04:28:58 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
Summary by CodeRabbit
New Features
JSONViewercomponent for rendering JSON dataRefactor
Chores