feat(web): plugin playground presets sidebar #1315
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
web/src/beta/features/PluginPlayground/Plugins/presets/ui/responsivePanel.ts (2)
4-23: Consider enhancing the plugin metadataThe YAML configuration could benefit from additional metadata fields such as:
description: To provide more context about the plugin's purposeauthor: To identify the maintainerrepository: To link to the source code
31-74: Improve CSS maintainability with custom propertiesConsider using CSS custom properties for colors and spacing to improve maintainability:
+ :root { + --color-text: #36454F; + --color-border: #ccc; + --color-background: #fff; + --spacing-unit: 8px; + --border-radius: 4px; + } li { - margin: 8px 0; - padding: 12px 16px; - color: #36454F; - background: #fff; - border: 1px solid #ccc; - border-radius: 4px; + margin: var(--spacing-unit) 0; + padding: calc(1.5 * var(--spacing-unit)) calc(2 * var(--spacing-unit)); + color: var(--color-text); + background: var(--color-background); + border: 1px solid var(--color-border); + border-radius: var(--border-radius);web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts (2)
7-8: Consider memoizing the flattened plugins array.While the current implementation works well for static presets, if presetPlugins becomes dynamic in the future, consider moving the flattening operation to a useMemo hook for better performance.
+ const flattenedPlugins = useMemo( + () => presetPlugins.map((category) => category.plugins).flat(), + [presetPlugins] + ); - const [plugins, setPlugins] = useState<PluginType[]>( - presetPlugins.map((category) => category.plugins).flat() - ); + const [plugins, setPlugins] = useState<PluginType[]>(flattenedPlugins);Also applies to: 12-14
Line range hint
1-214: Well-structured hook with good separation of concerns.The hook demonstrates good practices:
- Consistent error handling with notifications
- Proper memoization of callbacks
- Atomic file operations
- Clear separation between plugin and file management
Consider documenting the expected plugin structure with TypeScript interfaces or JSDoc comments for better maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
web/src/beta/features/PluginPlayground/Code/HtmlEditModal.tsx(2 hunks)web/src/beta/features/PluginPlayground/Code/hook.ts(1 hunks)web/src/beta/features/PluginPlayground/Code/index.tsx(0 hunks)web/src/beta/features/PluginPlayground/Console/index.tsx(2 hunks)web/src/beta/features/PluginPlayground/Plugins/FileListItem.tsx(2 hunks)web/src/beta/features/PluginPlayground/Plugins/constants.ts(0 hunks)web/src/beta/features/PluginPlayground/Plugins/index.tsx(4 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/common.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/custom/myPlugin.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/index.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/ui/responsivePanel.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/presets/ui/sidebar.ts(1 hunks)web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts(2 hunks)web/src/beta/features/PluginPlayground/hooks.tsx(4 hunks)
💤 Files with no reviewable changes (2)
- web/src/beta/features/PluginPlayground/Code/index.tsx
- web/src/beta/features/PluginPlayground/Plugins/constants.ts
✅ Files skipped from review due to trivial changes (1)
- web/src/beta/features/PluginPlayground/Plugins/presets/common.ts
🧰 Additional context used
📓 Learnings (5)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
web/src/beta/features/PluginPlayground/Console/index.tsx (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Console/index.tsx:19-21
Timestamp: 2024-12-03T03:50:19.816Z
Learning: In the `Console` component (`web/src/beta/features/PluginPlayground/Console/index.tsx`), when displaying outputs for each file, we intentionally use `{output || "✔"}` to display "✔" even when `output` is an empty string or any other falsy value.
web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
web/src/beta/features/PluginPlayground/hooks.tsx (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
web/src/beta/features/PluginPlayground/Plugins/index.tsx (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
🪛 Biome (1.9.4)
web/src/beta/features/PluginPlayground/Plugins/index.tsx
[error] 41-41: 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 (13)
web/src/beta/features/PluginPlayground/Plugins/presets/index.ts (1)
15-26: Structuring presetPlugins enhances plugin organization
The introduction of presetPlugins with categorized plugins improves the organization and scalability of the plugin system. This aligns with our future plans to support importing plugins and managing user-added plugins.
web/src/beta/features/PluginPlayground/Console/index.tsx (1)
19-21: Use of <Typography> and fallback output enhances UI consistency
Replacing the <div> with <Typography> improves semantic structure and UI consistency. Additionally, using {output || "✔"} ensures a default success indicator is displayed when output is falsy, which aligns with the intended design.
web/src/beta/features/PluginPlayground/Plugins/presets/custom/myPlugin.ts (1)
1-40: Addition of myPlugin preset is well-implemented
The new myPlugin preset is correctly defined with appropriate files and integrates properly with the preset plugins structure. This enhances the customization options available in the Plugin Playground.
web/src/beta/features/PluginPlayground/Code/HtmlEditModal.tsx (1)
25-25: LGTM! UI improvements enhance usability
The changes improve the modal's usability by:
- Increasing modal size for better HTML editing experience
- Adding consistent padding using theme spacing
Also applies to: 57-58
web/src/beta/features/PluginPlayground/Plugins/FileListItem.tsx (2)
7-7: Import path aligns with React hook naming conventions
The change from ./hook to ./usePlugins follows React's standard hook naming pattern, improving code maintainability.
97-97: UX improvement: Added cursor pointer style
Adding cursor pointer style improves user experience by visually indicating clickable items.
web/src/beta/features/PluginPlayground/hooks.tsx (2)
92-92: UI consistency: Capitalized tab name
Changed tab name from "code" to "Code" for consistent capitalization in the UI.
14-14: Consistent renaming from plugins to presetPlugins
The renaming aligns with the planned refactoring to support importing plugins in the future, as mentioned in the retrieved learning.
Also applies to: 62-62, 76-76
✅ Verification successful
The term 'plugins' is still being used appropriately in the codebase
The search results show that the remaining uses of 'plugins' are correct and serve different purposes than the renamed 'presetPlugins':
- In usePlugins.ts, 'plugins' represents the flattened array of active plugins derived from presetPlugins
- In presets/index.ts, 'plugins' is used as a property name in type definitions and category objects
- In hooks.tsx, 'plugins' is used as a tab identifier
The renaming from 'plugins' to 'presetPlugins' was specifically for the imported preset configuration, while other usages of 'plugins' remain semantically correct in their contexts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent renaming across codebase
rg -l 'plugins(?!.*preset)' web/src/beta/features/PluginPlayground/
Length of output: 299
Script:
#!/bin/bash
# Search for 'plugins' usage in the PluginPlayground directory
rg --pcre2 'plugins(?!.*preset)' web/src/beta/features/PluginPlayground/ -B 2 -A 2
# Also check for any remaining references to just 'plugins' in case the regex is too strict
rg '\bplugins\b' web/src/beta/features/PluginPlayground/ -B 2 -A 2
Length of output: 12915
web/src/beta/features/PluginPlayground/Plugins/presets/ui/responsivePanel.ts (1)
111-115: LGTM!
The plugin export is well-structured and follows the expected pattern.
web/src/beta/features/PluginPlayground/Code/hook.ts (1)
133-134: Verify impact on existing plugins
The new condition filters out non-widget extensions. While this change makes sense, we should verify that:
- All existing plugins are compatible with this change
- No functionality is broken for plugins that might have relied on non-widget extensions
✅ Verification successful
Let me analyze the results and generate a new script to better understand the extension types in the codebase.
The previous search revealed various type declarations but we need to:
- Identify which are top-level extension types vs property types
- Find how these types are used in the TypeScript code
- Understand if non-widget extensions are supported in the codebase
This will help us determine if filtering out non-widget extensions is a breaking change.
Let me try one more search to specifically look for extension type references in TypeScript files with a corrected syntax:
This will help us understand how extension types are used in the TypeScript code and whether filtering non-widget types could break existing functionality.
Based on the search results, I can now generate the final response. The codebase analysis shows that:
- The
typefield is used to distinguish different extension types including "widget", "primitive", "visualizer", "infobox", "block", "story", "storyBlock", and "cluster" - The TypeScript interfaces show that extensions are part of a plugin system where each extension has a specific type
- The filtering in the code is used in a widget-specific context (PluginPlayground/Code/hook.ts)
Filtering non-widget extensions is safe in this context
The condition if (cur.type !== "widget") return prv is appropriate because:
- It's in a widget-specific playground component
- The codebase shows clear separation between different extension types
- Other extension types (primitive, visualizer, etc.) are handled by their respective components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for extensions with types other than "widget"
echo "Searching for non-widget extensions in YAML files..."
rg -A 5 "type:\s*(?!widget)[a-zA-Z]+" --type yaml
# Search for potential extension type declarations
echo "Searching for extension type declarations..."
ast-grep --pattern 'type: $_'
Length of output: 28606
Script:
#!/bin/bash
# Search for extension type declarations in manifest files, focusing on top-level type fields
echo "Searching for top-level extension types..."
rg -A 1 '^\s+type:' --type yaml
# Search for any references to extension types in TypeScript files
echo "Searching for extension type references in TypeScript..."
rg -A 2 'type\s*[=:]\s*["\']?(widget|primitive|visualizer|infobox|block|story|storyBlock|cluster|infoboxBlock)' --type ts
Length of output: 56215
Script:
#!/bin/bash
# Search for extension type references in TypeScript files
echo "Searching for extension type references in TypeScript..."
rg -A 2 '"type":\s*"(widget|primitive|visualizer|infobox|block|story|storyBlock|cluster|infoboxBlock)"' --type ts
# Search for type-related imports and interfaces
echo "Searching for type definitions..."
rg -A 2 'interface.*Extension|type.*Extension' --type ts
Length of output: 38609
web/src/beta/features/PluginPlayground/Plugins/presets/ui/sidebar.ts (1)
4-23: Similar improvements needed as in responsivePanel.ts
web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts (2)
Line range hint 203-214: LGTM! Change aligns with future plans.
The switch to return presetPlugins instead of plugins is aligned with the planned refactoring to support plugin imports in the future, as noted in the retrieved learning.
17-19: Verify handling of empty plugin state.
The current initialization falls back to an empty string when no files exist. This could lead to issues if other parts of the code assume a valid file ID.
Let's verify how empty states are handled:
web/src/beta/features/PluginPlayground/Plugins/presets/ui/responsivePanel.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/beta/features/PluginPlayground/Plugins/index.tsx(1 hunks)web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts(3 hunks)
🧰 Additional context used
📓 Learnings (2)
web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
web/src/beta/features/PluginPlayground/Plugins/index.tsx (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
🔇 Additional comments (2)
web/src/beta/features/PluginPlayground/Plugins/index.tsx (1)
Line range hint 28-28: Add a key prop to the <div> element in the map function
In your code, the key prop is added to the CategoryTitle component, but the key prop should be on the <div> element directly inside the map function. This helps React identify which items have changed, are added, or are removed in a list.
Apply this diff to add the missing key prop:
{presetPlugins.map((category) => (
- <div>
+ <div key={category.id}>
<CategoryTitle>{category.title}</CategoryTitle>
{category.plugins.map((plugin) => (
<EntryItem
key={plugin.id}
highlighted={selectedPlugin.id === plugin.id}
onClick={() => selectPlugin(plugin.id)}
title={plugin.title}
optionsMenu={[
{
id: "0",
title: "share",
icon: "paperPlaneTilt",
onClick: () => handleShareIconClicked(plugin.id)
}
]}
optionsMenuWidth={100}
/>
))}
</div>
))}web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts (1)
3-4:
Remove duplicate imports from 'react'
The imports from 'react' are duplicated at lines 3 and 4. Consolidating them into a single import statement improves code clarity and prevents potential issues.
Apply this diff to consolidate the imports:
-import { useCallback, useMemo, useState } from "react";
-import { useCallback, useEffect, useMemo, useState } from "react";
+import { useCallback, useEffect, useMemo, useState } from "react";Likely invalid or redundant comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
web/src/beta/features/PluginPlayground/Plugins/index.tsx (3)
49-50: Remove redundant key prop from CategoryTitleThe
keyprop is already present on the parentdivelement. Having the samekeyon the childCategoryTitlecomponent is unnecessary.Apply this diff to remove the redundant key:
<div key={category.id}> - <CategoryTitle key={category.id}>{category.title}</CategoryTitle> + <CategoryTitle>{category.title}</CategoryTitle>
Line range hint
66-66: Add key prop to shared plugins containerFor consistency with the preset plugins section, add a key prop to the shared plugins container div.
Apply this diff:
- <div> + <div key="shared">
Line range hint
52-64: Refactor duplicated options menu configurationThe options menu configuration is duplicated between preset and shared plugins. Consider extracting this into a reusable constant.
Apply this refactor:
+ const PLUGIN_OPTIONS_MENU = (handleShare: (id: string) => void) => [{ + id: "0", + title: "share", + icon: "paperPlaneTilt", + onClick: (id: string) => handleShare(id) + }]; // Then in the render: <EntryItem key={plugin.id} highlighted={selectedPlugin.id === plugin.id} onClick={() => selectPlugin(plugin.id)} title={plugin.title} - optionsMenu={[ - { - id: "0", - title: "share", - icon: "paperPlaneTilt", - onClick: () => handleShareIconClicked(plugin.id) - } - ]} + optionsMenu={PLUGIN_OPTIONS_MENU(() => handleShareIconClicked(plugin.id))} optionsMenuWidth={100} />Also applies to: 69-81
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/beta/features/PluginPlayground/Plugins/index.tsx(1 hunks)
🧰 Additional context used
📓 Learnings (1)
web/src/beta/features/PluginPlayground/Plugins/index.tsx (1)
Learnt from: airslice
PR: reearth/reearth-visualizer#1299
File: web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts:203-203
Timestamp: 2024-12-03T03:49:26.713Z
Learning: In the `web/src/beta/features/PluginPlayground/Plugins/usePlugins.ts` file, returning `presetPlugins` instead of `plugins` is acceptable for now. We plan to refactor this code to return `plugins` when we add support for importing plugins, ensuring that the state includes user-added or modified plugins.
mulengawilfred
force-pushed
the
feat/plugin-playground-presets-sidebar
branch
from
9942f11 to
09a0de8
Compare
Co-authored-by: airslice <[email protected]>
Overview
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
Bug Fixes
Documentation