Conversation
|
WalkthroughThis PR extracts the core studio UI components and related utilities from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Extract the stage visualization system (Mockup, Choreography, LunaLynxStage, Studio) from apps/studio into a standalone reusable package. apps/studio is now a thin consumer of it.
fbfb54d to
ca44293
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/luna-stage-preview/src/components/studio-frame/studio-frame.tsx (1)
10-16: Consider clarifying non-interactive semantics in the prop API.Since the root class always includes
pointer-events-none, interactive handlers passed viarestPropswon’t fire. Consider either documenting that explicitly inStudioFramePropsor narrowing forwarded interactive props to avoid a misleading API surface.Also applies to: 24-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/luna-stage-preview/src/components/studio-frame/studio-frame.tsx` around lines 10 - 16, The StudioFrame component's root element always applies the CSS class "pointer-events-none", which prevents any interactive handlers forwarded via restProps from firing; update the StudioFrameProps type and usage to either (A) document this behavior by adding a clear comment to StudioFrameProps indicating the component is non-interactive and that event handlers passed via restProps will be ignored, or (B) change the forwarding logic to exclude interactive props (like onClick, onMouseDown, onKeyDown, tabIndex, role) from restProps so callers aren't misled—locate the StudioFrameProps definition and the root element where the "pointer-events-none" class and restProps are applied to implement one of these fixes.apps/studio/tailwind.config.ts (1)
10-14: Addsrcglob to Tailwind content config to prevent dev-time style drift from staledist.Studio's dev flow (
rsbuild dev) doesn't orchestrate luna-stage-preview builds, sodistcan lag during iteration. Scanningsrcas a fallback ensures class extraction stays current without requiring manual rebuilds.♻️ Suggested update
content: [ './src/**/*.{html,js,ts,jsx,tsx,mdx}', + '../../packages/luna-stage-preview/src/**/*.{js,ts,jsx,tsx,mdx}', // Scan compiled output of luna-stage-preview for Tailwind class names '../../packages/luna-stage-preview/dist/**/*.js', ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/studio/tailwind.config.ts` around lines 10 - 14, The Tailwind content array in tailwind.config.ts currently scans './src/**/*.{html,js,ts,jsx,tsx,mdx}' and the compiled '../../packages/luna-stage-preview/dist/**/*.js', but you need to add a fallback that scans the luna-stage-preview source to avoid dev-time style drift; update the content array to include the luna-stage-preview source glob (e.g., '../../packages/luna-stage-preview/src/**/*.{html,js,ts,jsx,tsx,mdx}') so Tailwind extracts class names from the package's src when dist is stale.packages/luna-stage-preview/src/components/lynx-stage/luna-lynx-stage.tsx (1)
69-86: Consider adding a return value for unhandled bridge calls.The
onNativeModulesCallhandler returns values forsetFocusedComponentandsetMoonriseState, but falls through without a return for other cases. If callers expect a response, this could cause issues.♻️ Optional: Add explicit return for unhandled cases
ref.current!.onNativeModulesCall = ( name, data, moduleName, ) => { if (moduleName === 'bridge') { if (name === 'setFocusedComponent') { const component: string = (data as { id: string }).id; onFocusedChange?.(component); return { entry, focusedComponent: component }; } else if (name === 'setMoonriseState') { onMoonriseChange?.(data as MoonriseEvent); return { entry, moonriseEvent: data as MoonriseEvent }; } } + return undefined; };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/luna-stage-preview/src/components/lynx-stage/luna-lynx-stage.tsx` around lines 69 - 86, The onNativeModulesCall handler assigned to ref.current (the onNativeModulesCall function) only returns for 'setFocusedComponent' and 'setMoonriseState' and leaves other bridge calls with no explicit return; update the handler to always return a defined value for unhandled cases (e.g., null or a standardized { entry } response) at the end of the function so callers receive a predictable response when moduleName === 'bridge' but name is not one of the handled values; ensure the change occurs inside the useEffect where ref.current!.onNativeModulesCall is set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/luna-stage-preview/package.json`:
- Line 45: Remove the unused dependency "@headlessui/tailwindcss" from this
package's package.json by deleting its entry from the dependencies section;
ensure you do not move it to devDependencies, run a quick grep across the
package source/build configs to confirm there are no remaining imports or
references to "@headlessui/tailwindcss", then run the package manager's install
to update the lockfile (or regenerate it) so the removal is reflected in the
lockfile.
In `@packages/luna-stage-preview/src/components/choreography/choreography.tsx`:
- Around line 14-27: The prop type declares viewMode as required
(StudioViewMode) but the Choreography component provides a default, so make the
prop optional to match the default: update the component props/type so viewMode
is viewMode?: StudioViewMode (instead of required), leaving the destructured
default in the Choreography function ({ viewMode = 'compare', ... }) intact;
modify any related usages or consumers if they relied on non-optional typing
(e.g., places importing the props type) to accept the optional prop.
---
Nitpick comments:
In `@apps/studio/tailwind.config.ts`:
- Around line 10-14: The Tailwind content array in tailwind.config.ts currently
scans './src/**/*.{html,js,ts,jsx,tsx,mdx}' and the compiled
'../../packages/luna-stage-preview/dist/**/*.js', but you need to add a fallback
that scans the luna-stage-preview source to avoid dev-time style drift; update
the content array to include the luna-stage-preview source glob (e.g.,
'../../packages/luna-stage-preview/src/**/*.{html,js,ts,jsx,tsx,mdx}') so
Tailwind extracts class names from the package's src when dist is stale.
In `@packages/luna-stage-preview/src/components/lynx-stage/luna-lynx-stage.tsx`:
- Around line 69-86: The onNativeModulesCall handler assigned to ref.current
(the onNativeModulesCall function) only returns for 'setFocusedComponent' and
'setMoonriseState' and leaves other bridge calls with no explicit return; update
the handler to always return a defined value for unhandled cases (e.g., null or
a standardized { entry } response) at the end of the function so callers receive
a predictable response when moduleName === 'bridge' but name is not one of the
handled values; ensure the change occurs inside the useEffect where
ref.current!.onNativeModulesCall is set.
In `@packages/luna-stage-preview/src/components/studio-frame/studio-frame.tsx`:
- Around line 10-16: The StudioFrame component's root element always applies the
CSS class "pointer-events-none", which prevents any interactive handlers
forwarded via restProps from firing; update the StudioFrameProps type and usage
to either (A) document this behavior by adding a clear comment to
StudioFrameProps indicating the component is non-interactive and that event
handlers passed via restProps will be ignored, or (B) change the forwarding
logic to exclude interactive props (like onClick, onMouseDown, onKeyDown,
tabIndex, role) from restProps so callers aren't misled—locate the
StudioFrameProps definition and the root element where the "pointer-events-none"
class and restProps are applied to implement one of these fixes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 663dc772-f880-41dc-8db0-4c77bd66a0ff
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (65)
.cspell/misc.txtapps/studio/package.jsonapps/studio/src/App.tsxapps/studio/src/components/choreography/archive/choreography-simple.tsxapps/studio/src/components/choreography/archive/compare-view.tsxapps/studio/src/components/choreography/archive/focus-view-test.tsxapps/studio/src/components/choreography/archive/focus-view.tsxapps/studio/src/components/choreography/choreography.tsxapps/studio/src/components/lynx-stage/lynx-stage.tsxapps/studio/src/components/mockup-motion/context/index.tsapps/studio/src/components/mockup-motion/context/use-visual-size.tsapps/studio/src/components/mockup-motion/index.tsapps/studio/src/components/studio-frame/index.tsapps/studio/src/components/studio-frame/studio-frame.tsxapps/studio/src/components/studio/index.tsapps/studio/src/components/studio/static.tsxapps/studio/src/components/studio/studio.tsxapps/studio/src/types/index.tsapps/studio/tailwind.config.tsapps/studio/tsconfig.jsonpackages/luna-stage-preview/package.jsonpackages/luna-stage-preview/rslib.config.tspackages/luna-stage-preview/src/components/choreography/choreography.tsxpackages/luna-stage-preview/src/components/choreography/dynamic-view.tsxpackages/luna-stage-preview/src/components/choreography/index.tspackages/luna-stage-preview/src/components/choreography/types.tspackages/luna-stage-preview/src/components/choreography/utils.tspackages/luna-stage-preview/src/components/container/container.tsxpackages/luna-stage-preview/src/components/container/index.tspackages/luna-stage-preview/src/components/context/create-context.tsxpackages/luna-stage-preview/src/components/context/index.tspackages/luna-stage-preview/src/components/lynx-stage/index.tspackages/luna-stage-preview/src/components/lynx-stage/luna-lynx-stage.tsxpackages/luna-stage-preview/src/components/lynx-stage/lynx-stage.tsxpackages/luna-stage-preview/src/components/menu-bar/index.tspackages/luna-stage-preview/src/components/menu-bar/menu-bar.tsxpackages/luna-stage-preview/src/components/mockup-motion/container-motion.tsxpackages/luna-stage-preview/src/components/mockup-motion/context/index.tspackages/luna-stage-preview/src/components/mockup-motion/context/use-visual-size.tspackages/luna-stage-preview/src/components/mockup-motion/context/visual-size-context.tsxpackages/luna-stage-preview/src/components/mockup-motion/context/visual-size-provider.tsxpackages/luna-stage-preview/src/components/mockup-motion/index.tspackages/luna-stage-preview/src/components/mockup-motion/mockup-motion.tsxpackages/luna-stage-preview/src/components/mockup-motion/presentation-motion.tsxpackages/luna-stage-preview/src/components/mockup-motion/transform-utils.tspackages/luna-stage-preview/src/components/mockup-motion/use-container-resize.tspackages/luna-stage-preview/src/components/mockup/index.tspackages/luna-stage-preview/src/components/mockup/mockup.tsxpackages/luna-stage-preview/src/components/studio-frame/index.tspackages/luna-stage-preview/src/components/studio-frame/studio-frame.tsxpackages/luna-stage-preview/src/components/studio/index.tspackages/luna-stage-preview/src/components/studio/studio.tsxpackages/luna-stage-preview/src/data/default-stages.tspackages/luna-stage-preview/src/hooks/use-container-resize.tspackages/luna-stage-preview/src/hooks/use-effect-event.tspackages/luna-stage-preview/src/hooks/use-isomorphic-layout-effect.tspackages/luna-stage-preview/src/hooks/use-merged-refs.tspackages/luna-stage-preview/src/hooks/use-raf-clock.tspackages/luna-stage-preview/src/index.tspackages/luna-stage-preview/src/types/global.d.tspackages/luna-stage-preview/src/types/stage.tspackages/luna-stage-preview/src/utils/index.tspackages/luna-stage-preview/tsconfig.build.jsonpackages/luna-stage-preview/tsconfig.jsontsconfig.json
💤 Files with no reviewable changes (15)
- packages/luna-stage-preview/src/utils/index.ts
- apps/studio/src/components/studio/index.ts
- apps/studio/src/components/mockup-motion/context/index.ts
- apps/studio/src/components/mockup-motion/index.ts
- apps/studio/src/components/studio-frame/index.ts
- apps/studio/src/components/mockup-motion/context/use-visual-size.ts
- apps/studio/src/components/lynx-stage/lynx-stage.tsx
- apps/studio/src/components/choreography/archive/choreography-simple.tsx
- apps/studio/src/components/studio/static.tsx
- apps/studio/src/components/studio-frame/studio-frame.tsx
- apps/studio/src/components/choreography/archive/focus-view-test.tsx
- apps/studio/src/components/choreography/archive/compare-view.tsx
- apps/studio/src/components/choreography/choreography.tsx
- apps/studio/src/components/choreography/archive/focus-view.tsx
- apps/studio/src/components/studio/studio.tsx
| "@dugyu/luna-catalog": "workspace:*", | ||
| "@dugyu/luna-core": "workspace:*", | ||
| "@headlessui/react": "^2.2.7", | ||
| "@headlessui/tailwindcss": "^0.2.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether `@headlessui/tailwindcss` is runtime-imported or build-tool-only.
set -euo pipefail
echo "== Runtime/source imports =="
rg -nP --iglob 'packages/luna-stage-preview/src/**/*.{ts,tsx,js,jsx,mjs,cjs}' \
"from\\s+['\"]@headlessui/tailwindcss['\"]|require\\(['\"]@headlessui/tailwindcss['\"]\\)" || true
echo "== Build/config imports =="
rg -nP --iglob 'packages/luna-stage-preview/**/*.{ts,tsx,js,jsx,mjs,cjs}' \
"@headlessui/tailwindcss" || trueRepository: Dugyu/lunarium
Length of output: 113
🏁 Script executed:
#!/bin/bash
# Search for tailwind config files in the package
find packages/luna-stage-preview -name "*tailwind*" -type f 2>/dev/null | head -20
# Search more broadly for any reference to the package across all file types
rg "@headlessui/tailwindcss" packages/luna-stage-preview/ -n --max-count 20
# Check package.json structure
echo "=== Package.json context ==="
cat packages/luna-stage-preview/package.json | grep -A 20 '"dependencies"' | head -30Repository: Dugyu/lunarium
Length of output: 866
🏁 Script executed:
#!/bin/bash
# Check if upstream workspace packages import `@headlessui/tailwindcss`
echo "=== Checking `@dugyu/luna-core` ==="
rg "@headlessui/tailwindcss" packages/luna-core/ -n 2>/dev/null | head -5 || echo "No matches found"
echo "=== Checking `@dugyu/luna-catalog` ==="
rg "@headlessui/tailwindcss" packages/luna-catalog/ -n 2>/dev/null | head -5 || echo "No matches found"
# Also check if it's listed in their dependencies
echo "=== luna-core package.json ==="
test -f packages/luna-core/package.json && grep -A 15 '"dependencies"' packages/luna-core/package.json | grep -E '(`@headlessui`|dependencies)' || echo "N/A"
echo "=== luna-catalog package.json ==="
test -f packages/luna-catalog/package.json && grep -A 15 '"dependencies"' packages/luna-catalog/package.json | grep -E '(`@headlessui`|dependencies)' || echo "N/A"Repository: Dugyu/lunarium
Length of output: 201
Remove @headlessui/tailwindcss from dependencies—it's not used anywhere in this package.
No imports of this package were found in the source code, build configs, or upstream workspace dependencies. If it was previously used and is no longer needed, it should be removed entirely rather than moved to devDependencies.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/luna-stage-preview/package.json` at line 45, Remove the unused
dependency "@headlessui/tailwindcss" from this package's package.json by
deleting its entry from the dependencies section; ensure you do not move it to
devDependencies, run a quick grep across the package source/build configs to
confirm there are no remaining imports or references to
"@headlessui/tailwindcss", then run the package manager's install to update the
lockfile (or regenerate it) so the removal is reflected in the lockfile.
| viewMode: StudioViewMode; | ||
| className?: string; | ||
| onThemeModeChange?: (mode: 'light' | 'dark') => void; | ||
| /** Override default stage configurations. */ | ||
| stages?: Record<string, StageMeta>; | ||
| /** Override default layout configs per view mode. */ | ||
| layouts?: Record<StudioViewMode, ViewSpec[]>; | ||
| /** Base URL for Lynx bundle files. @default '/' */ | ||
| bundleBaseUrl?: string; | ||
| }; | ||
|
|
||
| function Choreography( | ||
| { | ||
| viewMode = 'compare', |
There was a problem hiding this comment.
viewMode default and prop type are inconsistent.
viewMode is typed as required, but the function defines a default. Make it optional so callers can actually rely on the default.
Proposed fix
type ChoreographyProps = {
- viewMode: StudioViewMode;
+ viewMode?: StudioViewMode;
className?: string;
onThemeModeChange?: (mode: 'light' | 'dark') => void;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| viewMode: StudioViewMode; | |
| className?: string; | |
| onThemeModeChange?: (mode: 'light' | 'dark') => void; | |
| /** Override default stage configurations. */ | |
| stages?: Record<string, StageMeta>; | |
| /** Override default layout configs per view mode. */ | |
| layouts?: Record<StudioViewMode, ViewSpec[]>; | |
| /** Base URL for Lynx bundle files. @default '/' */ | |
| bundleBaseUrl?: string; | |
| }; | |
| function Choreography( | |
| { | |
| viewMode = 'compare', | |
| viewMode?: StudioViewMode; | |
| className?: string; | |
| onThemeModeChange?: (mode: 'light' | 'dark') => void; | |
| /** Override default stage configurations. */ | |
| stages?: Record<string, StageMeta>; | |
| /** Override default layout configs per view mode. */ | |
| layouts?: Record<StudioViewMode, ViewSpec[]>; | |
| /** Base URL for Lynx bundle files. `@default` '/' */ | |
| bundleBaseUrl?: string; | |
| }; | |
| function Choreography( | |
| { | |
| viewMode = 'compare', |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/luna-stage-preview/src/components/choreography/choreography.tsx`
around lines 14 - 27, The prop type declares viewMode as required
(StudioViewMode) but the Choreography component provides a default, so make the
prop optional to match the default: update the component props/type so viewMode
is viewMode?: StudioViewMode (instead of required), leaving the destructured
default in the Choreography function ({ viewMode = 'compare', ... }) intact;
modify any related usages or consumers if they relied on non-optional typing
(e.g., places importing the props type) to accept the optional prop.
Extract the stage visualization system (Mockup, Choreography, LunaLynxStage, Studio) from apps/studio into a standalone reusable package. apps/studio is now a thin consumer of it.
Summary by CodeRabbit
New Features
Refactor