-
Notifications
You must be signed in to change notification settings - Fork 8
feat: add examples/templates when adding a new tab #129
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
Conversation
80d67e5 to
f735af0
Compare
WalkthroughAdds a new examples module exporting a default array of three Marko example items: "counter", "let-localstorage", and "pointer-down". Refactors the playground tabs editor: replaces per-tab boolean editing with a numeric 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)**/*.marko📄 CodeRabbit inference engine (.cursorrules)
Files:
🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-11-25T20:17:04.100ZApplied to files:
📚 Learning: 2025-11-25T20:17:04.100ZApplied to files:
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting Comment |
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: 3
🤖 Fix all issues with AI agents
In `@src/routes/playground/tags/playground/tags/editor/tags/tabs/examples.ts`:
- Around line 41-44: The storage event handler handleStorageChange should guard
against null e.newValue before calling JSON.parse; update handleStorageChange to
first check that e.key === input.key and e.newValue is not null (or undefined)
and only then set internalValue = JSON.parse(e.newValue), otherwise set
internalValue to a safe fallback (e.g., null or {} as appropriate for the
component); reference the existing symbols handleStorageChange, input.key,
internalValue, and e.newValue when making this change.
In `@src/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.marko`:
- Around line 105-130: The blank-tab button click handler currently builds a
path using a count of tabs matching /^Tag\d*\.marko$/ which can collide after
deletions; change the logic in that handler (the onClick that sets newIndex,
tabs, selected, editingIndex) to compute the next available TagN index by
scanning existing tabs' paths (matching /^Tag(\d+)\.marko$/ and also "Tag.marko"
as index 0), choose the smallest non-used integer N, then create the new tab
with path `Tag${N}.marko` (or `Tag.marko` for N=0) and set selected and
editingIndex to the newIndex; keep tabs immutable (tabs = [...tabs, {...}]) and
preserve the other behavior (empty content).
- Around line 56-61: The key handler currently uses onKeyPress which is
deprecated and doesn't fire for non-printable keys like Escape; change the event
binding from onKeyPress to onKeyDown (keeping the same handler body that checks
e.key === "Enter" and e.key === "Escape") so the Escape branch reliably sets
editingIndex = -1 and Enter focuses the ".cm-content" element; update any
references to the handler in the Marko template (the inline ,onKeyPress(e) { ...
} block) to use ,onKeyDown(e) { ... } instead.
🧹 Nitpick comments (1)
src/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.style.module.scss (1)
59-65: Consider constraining popover height for long template lists.If the list grows, the popover can exceed the viewport. Adding a max-height with scrolling keeps it usable.
♻️ Suggested tweak
.popover { inset: unset; max-width: 95vw; + max-height: 80vh; + overflow: auto; background-color: var(--color-gray-alt-dim); border: 1px solid var(--color-gray-alt); border-radius: 0.25rem; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/routes/playground/tags/playground/tags/editor/tags/tabs/examples.tssrc/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.markosrc/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.style.module.scss
🧰 Additional context used
📓 Path-based instructions (1)
**/*.marko
📄 CodeRabbit inference engine (.cursorrules)
In Marko files, use JS-style comments (
// commentor/* comment */) instead of HTML comments (<!-- comment -->)
Files:
src/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.marko
🧠 Learnings (1)
📚 Learning: 2025-11-25T20:17:04.100Z
Learnt from: CR
Repo: marko-js/website PR: 0
File: .cursorrules:0-0
Timestamp: 2025-11-25T20:17:04.100Z
Learning: Highlight zero-JS by default and progressive enhancement in Marko documentation
Applied to files:
src/routes/playground/tags/playground/tags/editor/tags/tabs/examples.ts
🔇 Additional comments (5)
src/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.style.module.scss (2)
38-49: Add-button styling change looks consistent.No issues spotted.
67-77: Template button hover/focus styling looks good.src/routes/playground/tags/playground/tags/editor/tags/tabs/examples.ts (1)
1-19: Counter template is clear and self-contained.src/routes/playground/tags/playground/tags/editor/tags/tabs/tabs.marko (2)
4-41: Editing-index state wiring reads clean.The
editingIndexapproach keeps selection/editing logic straightforward.
81-103: Popover positioning and focus-out close wiring look solid.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
video.mov