-
Notifications
You must be signed in to change notification settings - Fork 3
Fix playground #189
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
Fix playground #189
Conversation
- Added VideoDemo component - Added arkenv-video.mov - Updated home page
|
|
Warning Rate limit exceeded@yamcodes has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 55 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughReplaces the homepage StackBlitz embed with a VideoDemo, removes StackBlitz-specific middleware, env var and dependency, updates StarUsButton to derive owner/repo from a GitHub URL, reorders NODE_ENV in a playground config, and adds an example dev script and StackBlitz startCommand. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant HP as HomePage
participant VD as VideoDemo (client)
participant B as Browser
participant SB as stackblitz.com
U->>HP: Request "/"
HP->>VD: Render VideoDemo client component
VD->>B: Autoplay muted looping video (teaser)
Note over VD,B: Visual teaser shown
U->>VD: Click video/button
VD->>B: window.open(StackBlitz URL, "_blank", "noopener,noreferrer")
B->>SB: Open StackBlitz example in new tab
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
- Removed StackBlitz demo component - Removed middleware for StackBlitz - Removed env var and sdk dependency
- Reduced dark mode shadow intensity
- Wrapped video in a button - Allows play on click
- Removed embed params - Removed duplicated source
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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: 2
🧹 Nitpick comments (2)
examples/basic/package.json (1)
23-24: Simplify StackBlitz startCommandStackBlitz installs deps automatically. Dropping the explicit npm install speeds startup and avoids duplicate installs.
Apply this diff:
- "startCommand": "npm install && npm run dev:example" + "startCommand": "npm run dev:example"apps/www/components/page/video-demo.tsx (1)
21-27: Optional: reduce load and improve UX for motion-sensitive usersConsider a poster and lighter preload to improve performance; optionally pause autoplay for prefers-reduced-motion.
Example:
- <video - autoPlay - loop - muted - playsInline - className="block max-h-[600px] sm:max-h-[1000px] object-contain" - > + <video + autoPlay + loop + muted + playsInline + preload="metadata" + poster="/arkenv-video-poster.jpg" + className="block max-h-[600px] sm:max-h-[1000px] object-contain" + >Optional in JS:
// Pause for users who prefer reduced motion useEffect(() => { const m = window.matchMedia("(prefers-reduced-motion: reduce)"); const el = document.querySelector("video"); if (!el) return; const apply = () => (m.matches ? el.pause() : el.play().catch(() => {})); apply(); m.addEventListener?.("change", apply); return () => m.removeEventListener?.("change", apply); }, []);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
apps/www/public/arkenv-video.movis excluded by!**/*.movpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/playgrounds/node/index.ts(1 hunks)apps/www/.env.example(0 hunks)apps/www/app/(home)/page.tsx(2 hunks)apps/www/components/page/index.ts(1 hunks)apps/www/components/page/stackblitz-demo.tsx(0 hunks)apps/www/components/page/star-us-button.tsx(2 hunks)apps/www/components/page/video-demo.tsx(1 hunks)apps/www/middleware.ts(0 hunks)apps/www/package.json(1 hunks)examples/basic/package.json(2 hunks)
💤 Files with no reviewable changes (3)
- apps/www/.env.example
- apps/www/middleware.ts
- apps/www/components/page/stackblitz-demo.tsx
🧰 Additional context used
🧬 Code graph analysis (5)
apps/www/components/page/star-us-button.tsx (2)
apps/www/lib/utils/github.test.ts (2)
process(134-144)getLinkTitleAndHref(149-149)apps/www/lib/utils/github.ts (2)
path(25-32)githubUrl(7-16)
apps/playgrounds/node/index.ts (1)
packages/arkenv/src/type.test.ts (4)
envType(80-102)envType(76-76)envType(123-123)envType(5-9)
apps/www/components/page/video-demo.tsx (1)
apps/www/components/page/stackblitz-demo.tsx (2)
StackBlitzDemo(6-34)embedRef(9-27)
apps/www/components/page/index.ts (1)
apps/www/components/page/stackblitz-demo.tsx (1)
StackBlitzDemo(6-34)
apps/www/app/(home)/page.tsx (1)
apps/www/components/page/video-demo.tsx (1)
VideoDemo(3-35)
🔇 Additional comments (7)
apps/playgrounds/node/index.ts (1)
6-6: LGTM: Reordering only; no behavioral change.The NODE_ENV union-with-default definition looks correct for arkenv’s DSL. No issues spotted.
apps/www/components/page/index.ts (1)
5-5: LGTM – correct re-exportExporting video-demo aligns with the homepage changes; StackBlitz export removal is reflected here.
apps/www/app/(home)/page.tsx (2)
3-3: LGTM – updated importSwitch to VideoDemo matches the new component export.
44-46: LGTM – layout tweakSpacing and VideoDemo placement look good.
apps/www/components/page/video-demo.tsx (1)
28-29: Fix incorrect MIME types and sourcesThe same .mov file is declared as video/mp4, which can break playback. Either remove the incorrect source or provide real MP4/WEBM assets.
Minimal safe fix:
- <source src="/https/github.com/arkenv-video.mov" type="video/quicktime" /> - <source src="/https/github.com/arkenv-video.mov" type="video/mp4" /> + <source src="/https/github.com/arkenv-video.mov" type="video/quicktime" />Recommended (if you add assets):
<video ...> <source src="/arkenv-video.webm" type="video/webm" /> <source src="/arkenv-video.mp4" type="video/mp4" /> <source src="/arkenv-video.mov" type="video/quicktime" /> Your browser does not support the video tag. </video>examples/basic/package.json (1)
8-8: Confirm examples/basic/.env.example is committed
dev:example depends on examples/basic/.env.example; sandbox search returned no match — add/commit examples/basic/.env.example so StackBlitz/local runs don't fail.apps/www/package.json (1)
26-26: No @stackblitz/sdk imports remain — removal OK. Found no imports/usages of @stackblitz/sdk; only a hardcoded StackBlitz URL (apps/www/components/page/video-demo.tsx) and a "stackblitz" field in examples/basic/package.json remain — neither uses the SDK.
- Use env var for github repo - Computed once, avoids repetition - Added github url breakdown util
- Added aria-label to the play button for the demo video to improve accessibility.
Summary by CodeRabbit
New Features
Chores