-
Notifications
You must be signed in to change notification settings - Fork 46
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(web): plugin playground / display console #1263
Conversation
This reverts commit 02ce6eb.
…into playground/display-code-editor
…alizer into playground/display-code-editor
…eearth/reearth-visualizer into playground/reflect-code-to-visual
…eearth/reearth-visualizer into playground/reflect-code-to-visual
…b.com/reearth/reearth-visualizer into playground/reflect-code-to-visual
…into playground/display-code-editor
…eearth/reearth-visualizer into playground/reflect-code-to-visual
WalkthroughThe changes in this pull request enhance the functionality of the Changes
Possibly related PRs
Suggested reviewers
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
|
✅ Deploy Preview for reearth-web canceled.
|
…into playground/display-console
…into playground/display-console
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (5)
web/src/beta/features/PluginPlayground/Console/index.tsx (3)
4-10
: Consider enhancing type definitions for better type safety.The type definition is good, but could be improved for better maintainability and type safety.
Consider this enhancement:
type Props = { fileOutputs: | { + id: string; // Add unique identifier for React keys title: string; - output: string; + output: string | Error; // Support error objects + timestamp?: number; // Add timestamp for sorting }[] | undefined; };
26-32
: Enhance console styling for better usability.The current styling could be improved for better readability and cross-browser compatibility.
Consider these style enhancements:
const Wrapper = styled.div(() => ({ display: "flex", flexDirection: "column", maxHeight: "100%", height: "100%", - overflow: "auto" + overflow: "auto", + padding: "8px", + wordBreak: "break-word", + fontFamily: "monospace", + backgroundColor: "#f5f5f5", + "&::-webkit-scrollbar": { + width: "8px", + }, + "&::-webkit-scrollbar-thumb": { + backgroundColor: "#888", + borderRadius: "4px", + } }));
1-33
: Consider adding common console features.To better align with typical development consoles (like the Cesium sandbox mentioned in PR objectives), consider adding these features:
- Log levels (info, warn, error)
- Clear console button
- Filter by log type
- Copy output to clipboard
- Console command input
- Collapsible output groups
Would you like me to provide implementation examples for any of these features?
🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
web/src/beta/features/PluginPlayground/hooks.tsx (1)
Line range hint
27-52
: Consider implementing output management strategiesFor a robust plugin playground implementation, consider:
- Implementing output size limits to prevent memory issues
- Adding output clearing functionality
- Adding timestamp information to outputs
- Implementing output persistence across sessions
Would you like me to provide specific implementation suggestions for any of these improvements?
web/src/beta/features/PluginPlayground/Code/hook.ts (1)
96-113
: Improve error handling specificityThe current error handling could be more specific about different types of errors that might occur during execution.
Consider this enhanced error handling:
try { return { title: file.title, output: fn() }; } catch (error) { if (error instanceof SyntaxError) { return { title: file.title, output: `Syntax Error: ${error.message}` }; } else if (error instanceof ReferenceError) { return { title: file.title, output: `Reference Error: ${error.message}` }; } else if (error instanceof Error) { return { title: file.title, output: `Runtime Error: ${error.message}` }; } return { title: file.title, output: "Unknown execution error occurred" }; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
web/src/beta/features/PluginPlayground/Code/hook.ts
(3 hunks)web/src/beta/features/PluginPlayground/Console/index.tsx
(1 hunks)web/src/beta/features/PluginPlayground/hooks.tsx
(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
web/src/beta/features/PluginPlayground/Console/index.tsx
[error] 18-18: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (5)
web/src/beta/features/PluginPlayground/hooks.tsx (3)
49-49
: LGTM: Console component integration
The fileOutputs prop is correctly passed to the Console component, implementing the core feature for log display.
52-52
: LGTM: Proper React hooks usage
The dependency array is correctly updated to include fileOutputs, ensuring the component re-renders when outputs change.
27-29
: Verify error handling in useCode hook
The integration looks correct, but let's ensure proper error handling for the file execution outputs.
✅ Verification successful
Error handling is properly implemented in the codebase
The verification shows comprehensive error handling in the code execution flow:
- The
useCode
hook and related components have proper error handling:- YAML parsing errors are caught and provide user-friendly messages
- Code execution errors are caught and displayed in file outputs
- Console errors are properly captured and displayed
- Error states are properly propagated through notifications
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check error handling implementation in useCode hook
# Look for try-catch blocks and error state management
ast-grep --pattern 'try {
$$$
} catch (error) {
$$$
}'
# Check for error-related state management
rg -A 3 'error|Error' ./web/src/beta/features/PluginPlayground/Code/hook.ts
Length of output: 18043
web/src/beta/features/PluginPlayground/Code/hook.ts (2)
54-59
: LGTM! Well-structured state definition.
The type definition for fileOutputs
is clear and appropriate for storing console outputs with their associated file titles.
182-183
: LGTM! Proper exposure of the new state.
The addition of fileOutputs
to the return object is consistent with React hooks pattern and allows parent components to access the console output data.
…into playground/display-console
…into playground/display-console
Thank you for the review. I'll merge it. |
Overview
I have added a process to display logs in the console during code execution, referring to the sandbox of Cesium.
What I've done
What I haven't done
How I tested
Which point I want you to review particularly
Memo
Summary by CodeRabbit
New Features
fileOutputs
prop to the Console component for displaying execution results.Wrapper
component for improved output display.Bug Fixes
Refactor
useCode
hook and related components for better data flow.