feat(scaffold-examples/slidev): create app #251
Conversation
📝 WalkthroughWalkthroughThe pull request introduces a comprehensive setup for a Slidev presentation project within the scaffold-examples directory. The changes include configuration files for various deployment platforms (Netlify, Vercel), package management settings, and a complete slide deck template. The project is configured with Vue.js and Slidev, providing a ready-to-use presentation environment with interactive components, custom styling, and deployment configurations. Changes
Suggested Reviewers
Poem
Finishing Touches
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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
apps/scaffold-examples/slidev/vercel.json (1)
1-7: Consider unifying deployment configurations.Having separate configuration files for Vercel and Netlify could lead to maintenance overhead. Consider using a unified deployment configuration approach:
- Create a shared build configuration that both platforms can use
- Use environment variables for platform-specific settings
- Document the deployment process for both platforms in a single place
apps/scaffold-examples/slidev/snippets/external.ts (2)
5-7: Consider initializing array elementsThe
emptyArrayfunction creates an array with undefined elements. Consider providing an optional initialization value.-export function emptyArray<T>(length: number) { - return Array.from<T>({ length }) +export function emptyArray<T>(length: number, initialValue?: T) { + return Array.from<T>({ length }, () => initialValue as T) +}
10-12: Consider adding JSDoc commentsAdd documentation to explain the purpose of this utility function.
+/** + * Logs a greeting message indicating the source file. + */ export function sayHello() { console.log('Hello from snippets/external.ts') }apps/scaffold-examples/slidev/components/Counter.vue (2)
4-8: Add type and validation to count propThe count prop should have a type and validation for better type safety and runtime checks.
const props = defineProps({ count: { + type: Number, default: 0, + validator: (value: number) => Number.isFinite(value) }, })
14-36: Add ARIA labels for better accessibilityThe increment/decrement buttons should have ARIA labels for screen readers.
<button border="r main" p="2" font="mono" outline="!none" hover:bg="gray-400 opacity-20" + aria-label="Decrement counter" @click="counter -= 1" > - </button> <span m="auto" p="2">{{ counter }}</span> <button border="l main" p="2" font="mono" outline="!none" hover:bg="gray-400 opacity-20" + aria-label="Increment counter" @click="counter += 1" > + </button>apps/scaffold-examples/slidev/slides.md (1)
94-94: Fix grammar in navigation instructionChange "Hover on the bottom-left corner" to "Hover in the bottom-left corner" for correct preposition usage.
🧰 Tools
🪛 LanguageTool
[grammar] ~94-~94: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the bottom-left corner”?
Context: ...de-up level: 2 --- # Navigation Hover on the bottom-left corner to see the navigation's controls panel,...(ON_IN_THE_CORNER_2)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/scaffold-examples/slidev/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
apps/scaffold-examples/slidev/.gitignore(1 hunks)apps/scaffold-examples/slidev/.npmrc(1 hunks)apps/scaffold-examples/slidev/README.md(1 hunks)apps/scaffold-examples/slidev/components/Counter.vue(1 hunks)apps/scaffold-examples/slidev/netlify.toml(1 hunks)apps/scaffold-examples/slidev/package.json(1 hunks)apps/scaffold-examples/slidev/pages/imported-slides.md(1 hunks)apps/scaffold-examples/slidev/slides.md(1 hunks)apps/scaffold-examples/slidev/snippets/external.ts(1 hunks)apps/scaffold-examples/slidev/vercel.json(1 hunks)
✅ Files skipped from review due to trivial changes (4)
- apps/scaffold-examples/slidev/README.md
- apps/scaffold-examples/slidev/.npmrc
- apps/scaffold-examples/slidev/netlify.toml
- apps/scaffold-examples/slidev/.gitignore
🧰 Additional context used
🪛 Markdownlint (0.37.0)
apps/scaffold-examples/slidev/pages/imported-slides.md
5-5: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
apps/scaffold-examples/slidev/slides.md
563-563: Expected: h2; Actual: h6
Heading levels should only increment by one level at a time
(MD001, heading-increment)
47-47: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
88-88: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
115-115: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
134-134: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
188-188: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
304-304: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
552-552: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
597-597: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
628-628: Expected: atx; Actual: setext
Heading style
(MD003, heading-style)
135-135: null
Bare URL used
(MD034, no-bare-urls)
586-586: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
🪛 LanguageTool
apps/scaffold-examples/slidev/slides.md
[grammar] ~94-~94: The usual preposition to use with “corner” is “in”, not “on”. Did you mean “in the bottom-left corner”?
Context: ...de-up level: 2 --- # Navigation Hover on the bottom-left corner to see the navigation's controls panel,...
(ON_IN_THE_CORNER_2)
[uncategorized] ~128-~128: Loose punctuation mark.
Context: ...itleandlevel` in your frontmatter. ::right:: <Toc text-sm minDepth="1" maxD...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (3)
apps/scaffold-examples/slidev/package.json (1)
1-16: Add viewport testing configurationThe PR objectives mention ensuring tests pass across different screen sizes (mobile, tablet, desktop). Consider adding Vitest UI or Playwright configuration for viewport testing.
Would you like me to provide an example configuration for viewport testing?
apps/scaffold-examples/slidev/vercel.json (2)
5-6: Verify build script configuration.The build command references "npm run build". Let's ensure this script is properly configured in your package.json.
Run this script to verify the build configuration:
✅ Verification successful
Build configuration is correctly set up ✅
The build command and output directory in vercel.json are properly configured for this Slidev project:
npm run buildcorrectly maps to theslidev buildscript in package.json- The
distoutput directory is properly gitignored🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify build script configuration # Check if package.json exists and contains the build script fd -t f "package.json" -x jq -r '.scripts.build // empty' # Check if the dist directory is properly gitignored rg -l "^dist/?$" .gitignoreLength of output: 173
Script:
#!/bin/bash # Check the package.json in the same directory as vercel.json cat apps/scaffold-examples/slidev/package.json | jq -r '.scripts' # Verify if this is indeed a Slidev project fd -p "apps/scaffold-examples/slidev/slides.*"Length of output: 239
2-4: Verify static asset handling with the catch-all rewrite rule.While the catch-all rewrite rule is necessary for client-side routing, ensure that static assets (images, fonts, etc.) are served correctly and not unnecessarily rewritten to index.html.
Run this script to check for static asset references in your Slidev project:
✅ Verification successful
The catch-all rewrite rule is safe for this Slidev implementation
The implementation only uses external assets (from sli.dev and GitHub), with no local static assets that could be affected by the rewrite rule. The catch-all rewrite to index.html will correctly handle client-side routing without interfering with asset serving.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for static asset references that might be affected by the rewrite rule # Look for static asset references in the project rg -g '!{dist,node_modules}' -t html -t vue -t js -t ts '\.(jpg|jpeg|png|gif|svg|woff2?|ttf|eot)' # Check if there's a public directory with static assets fd -t d '^public$'Length of output: 412
Script:
#!/bin/bash # Search for asset references without file type restrictions in Slidev directory cd apps/scaffold-examples/slidev && \ rg -g '!{dist,node_modules}' '\.(jpg|jpeg|png|gif|svg|woff2?|ttf|eot)' . # Check for Slidev-specific asset directories and configuration ls -la apps/scaffold-examples/slidev/ # Look for any asset-related configuration in package.json cat apps/scaffold-examples/slidev/package.jsonLength of output: 989
| @@ -0,0 +1,16 @@ | |||
| { | |||
| "name": "slidev", | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use a more specific package name
The current package name "slidev" is too generic and could potentially conflict with the actual Slidev package. Consider using a more specific name that reflects its purpose as an example/scaffold, such as "@scaffold-examples/slidev" or "slidev-scaffold-example".
- "name": "slidev",
+ "name": "@scaffold-examples/slidev",📝 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.
| "name": "slidev", | |
| "name": "@scaffold-examples/slidev", |
| "@slidev/theme-default": "latest", | ||
| "@slidev/theme-seriph": "latest", |
There was a problem hiding this comment.
Avoid using 'latest' tag for dependencies
Using "latest" for theme packages can lead to unexpected breaks when new versions are released. It's better to specify exact versions or version ranges.
- "@slidev/theme-default": "latest",
- "@slidev/theme-seriph": "latest",
+ "@slidev/theme-default": "^0.25.0",
+ "@slidev/theme-seriph": "^0.25.0",📝 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.
| "@slidev/theme-default": "latest", | |
| "@slidev/theme-seriph": "latest", | |
| "@slidev/theme-default": "^0.25.0", | |
| "@slidev/theme-seriph": "^0.25.0", |
| "@slidev/cli": "^51.0.0", | ||
| "@slidev/theme-default": "latest", | ||
| "@slidev/theme-seriph": "latest", | ||
| "vue": "^3.5.13" |
There was a problem hiding this comment.
Fix incorrect Vue.js version
The specified Vue.js version (3.5.13) doesn't exist yet. The latest stable version is in the 3.4.x range.
- "vue": "^3.5.13"
+ "vue": "^3.4.0"📝 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.
| "vue": "^3.5.13" | |
| "vue": "^3.4.0" |
| "scripts": { | ||
| "build": "slidev build", | ||
| "dev": "slidev --open", | ||
| "export": "slidev export" | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add essential development scripts
The current scripts configuration lacks some essential development tools:
- No test script despite PR objectives mentioning test coverage
- No lint/format scripts for code quality
Consider adding these scripts:
"scripts": {
"build": "slidev build",
"dev": "slidev --open",
- "export": "slidev export"
+ "export": "slidev export",
+ "test": "vitest",
+ "lint": "eslint .",
+ "format": "prettier --write ."
},Committable suggestion skipped: line range outside the PR's diff.
| "dependencies": { | ||
| "@slidev/cli": "^51.0.0", | ||
| "@slidev/theme-default": "latest", | ||
| "@slidev/theme-seriph": "latest", | ||
| "vue": "^3.5.13" | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add development dependencies
The package is missing essential development dependencies for testing, linting, and formatting that would be needed for the scripts suggested earlier.
Add a devDependencies section:
"dependencies": {
"@slidev/cli": "^51.0.0",
"@slidev/theme-default": "^0.25.0",
"@slidev/theme-seriph": "^0.25.0",
"vue": "^3.4.0"
+ },
+ "devDependencies": {
+ "vitest": "^1.0.0",
+ "eslint": "^8.0.0",
+ "prettier": "^3.0.0"
}📝 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.
| "dependencies": { | |
| "@slidev/cli": "^51.0.0", | |
| "@slidev/theme-default": "latest", | |
| "@slidev/theme-seriph": "latest", | |
| "vue": "^3.5.13" | |
| } | |
| "dependencies": { | |
| "@slidev/cli": "^51.0.0", | |
| "@slidev/theme-default": "latest", | |
| "@slidev/theme-seriph": "latest", | |
| "vue": "^3.5.13" | |
| }, | |
| "devDependencies": { | |
| "vitest": "^1.0.0", | |
| "eslint": "^8.0.0", | |
| "prettier": "^3.0.0" | |
| } |
|
|
||
| You can split your slides.md into multiple files and organize them as you want using the `src` attribute. | ||
|
|
||
| #### `slides.md` |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix heading hierarchy
The heading levels should only increment by one level at a time. Change h4 headings to h2 for proper document structure.
-#### `slides.md`
+## `slides.md`
...
-#### `subpage.md`
+## `subpage.md`Also applies to: 19-19
🧰 Tools
🪛 Markdownlint (0.37.0)
5-5: Expected: h2; Actual: h4
Heading levels should only increment by one level at a time
(MD001, heading-increment)
| </div> | ||
| </v-drag> | ||
|
|
||
| <img v-drag="'square'" src="https://sli.dev/logo.png"> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add alt text to logo image
Images should have descriptive alt text for accessibility.
-<img v-drag="'square'" src="https://sli.dev/logo.png">
+<img v-drag="'square'" src="https://sli.dev/logo.png" alt="Slidev logo">📝 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.
| <img v-drag="'square'" src="https://sli.dev/logo.png"> | |
| <img v-drag="'square'" src="https://sli.dev/logo.png" alt="Slidev logo"> |
🧰 Tools
🪛 Markdownlint (0.37.0)
586-586: null
Images should have alternate text (alt text)
(MD045, no-alt-text)
Checklist
Summary by CodeRabbit
New Features
Documentation
Configuration
.gitignoreto exclude unnecessary files