Custom cursor #45
Custom cursor #45
Conversation
WalkthroughThe changes in this pull request include updates to the Changes
Possibly related PRs
🪧 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 (
|
Deploying svelte-maplibre-gl with
|
| Latest commit: |
0b3c250
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://9d3de007.svelte-maplibre-gl.pages.dev |
| Branch Preview URL: | https://cursor.svelte-maplibre-gl.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (9)
src/content/examples/cursor/content.svelte.md (3)
1-4: Consider enhancing the description with more details.While the current description is clear, it could be more informative by including:
- The specific cursor styles that can be changed
- When the hover effect occurs (e.g., over specific map features)
- Any configuration options available
--- title: Change Cursor -description: Change the cursor style on hover +description: Customize the map cursor style when hovering over different map features. Supports standard CSS cursor values and custom cursors. ---
9-9: Consider using module aliases for imports.The relative import path
../../CodeBlock.sveltecould be fragile if the file structure changes. Consider using module aliases (e.g.,$lib,$components) for more maintainable imports.-import CodeBlock from "../../CodeBlock.svelte"; +import CodeBlock from "$components/CodeBlock.svelte";
13-15: Consider adding usage context before the demo.To improve the learning experience, consider adding a brief explanation or context before the demo component. This could help users understand what to expect and how to interact with the demo.
+<p> + This example demonstrates how to customize the map cursor. Try hovering over different + parts of the map to see the cursor change. +</p> + <Demo /> <CodeBlock content={demoRaw} {shiki} />src/routes/components/[slug]/+page.ts (1)
9-13: Consider memoizing the highlighter initializationThe highlighter is currently initialized on every module import. Consider memoizing this initialization to improve performance, especially since the configuration is static.
-const shiki = createHighlighter({ +const shiki = browser ? await createHighlighter({ themes: [dark], langs: [svelte], engine: browser ? createOnigurumaEngine(import('shiki/wasm')) : createJavaScriptRegexEngine() -}); +}) : null;src/content/examples/cursor/Cursor.svelte (2)
5-26: Consider extracting constants to a separate file.The
CURSORSarray andcentercoordinates could be useful in other components. Consider moving them to a separate constants file for better reusability and maintainability.+// src/lib/constants/map.ts +export const MAP_CENTER = [0, 0]; +export const CURSOR_STYLES = [ + 'cell', + 'col-resize', + // ... rest of the cursors +]; -const center = [0, 0]; -const CURSORS = [ - 'cell', - 'col-resize', - // ... rest of the cursors -]; +import { MAP_CENTER as center, CURSOR_STYLES as CURSORS } from '$lib/constants/map';
47-67: Consider making styles more configurable.The circle styling and container height could be made more flexible:
- Extract circle paint properties to a constant or make them configurable via props
- Consider using more responsive height classes
-class="h-[55vh] min-h-[300px]" +class="h-screen min-h-[300px] max-h-[800px]" +const circlePaint = { + 'circle-radius': 12, + 'circle-color': '#007cbf', + 'circle-stroke-color': '#fff', + 'circle-stroke-width': 3 +} as const; -paint={{ - 'circle-radius': 12, - 'circle-color': '#007cbf', - 'circle-stroke-color': '#fff', - 'circle-stroke-width': 3 -}} +paint={circlePaint}🧰 Tools
🪛 eslint
[error] 58-58: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
src/lib/maplibre/map/MapLibre.svelte (3)
21-21: Add JSDoc documentation for the cursor propertyConsider adding JSDoc documentation to describe the expected cursor values and their effects.
+ /** Custom cursor style to be applied to the map canvas. Accepts standard CSS cursor values. */ cursor?: string;
201-204: Optimize cursor initializationThe cursor initialization can be simplified and the code duplication with the reactive effect can be eliminated.
- if (cursor) { - map.getCanvas().style.cursor = cursor ?? ''; - }The initial cursor setup can be handled by the reactive effect that follows, avoiding code duplication.
316-321: Improve cursor effect implementationThe cursor effect implementation can be enhanced for better clarity and efficiency.
$effect(() => { - cursor; - if (map && !firstRun) { - map.getCanvas().style.cursor = cursor ?? ''; - } + if (!map) return; + map.getCanvas().style.cursor = cursor ?? ''; });Changes:
- Removed redundant cursor tracking line as $effect automatically tracks it
- Simplified the condition as firstRun check isn't necessary for cursor updates
- Early return pattern for better readability
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
README.md(2 hunks)src/content/examples/cursor/Cursor.svelte(1 hunks)src/content/examples/cursor/content.svelte.md(1 hunks)src/lib/maplibre/map/MapLibre.svelte(4 hunks)src/routes/components/[slug]/+page.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- README.md
🧰 Additional context used
🪛 eslint
src/content/examples/cursor/Cursor.svelte
[error] 28-28: 'GeoJSON' is not defined.
(no-undef)
[error] 58-58: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
🔇 Additional comments (5)
src/content/examples/cursor/content.svelte.md (1)
9-10: Verify CodeBlock component implementation.
Let's ensure the CodeBlock component exists and follows the same pattern across the codebase.
✅ Verification successful
CodeBlock component and shiki prop usage verified
The CodeBlock component exists at src/content/CodeBlock.svelte and is properly implemented with the expected shiki prop. The component is consistently imported and used across multiple example files in the codebase, with all instances following the same pattern of importing CodeBlock and destructuring the shiki prop from $props(). The implementation matches the usage in the file under review.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify CodeBlock component existence and usage pattern
# Check if CodeBlock component exists
fd CodeBlock.svelte
# Check the usage pattern of CodeBlock with shiki prop
rg -A 2 "import CodeBlock.*from" --type svelte
Length of output: 130
Script:
#!/bin/bash
# Let's verify the CodeBlock component and its usage with different approaches
# Check the content of CodeBlock component
cat src/content/CodeBlock.svelte
# Search for CodeBlock imports and usage across all files
rg "import CodeBlock"
# Search for shiki prop usage
rg "shiki.*props"
Length of output: 5167
src/routes/components/[slug]/+page.ts (1)
19-19: Verify the impact of changing content path from 'examples' to 'components'
This change could affect existing routes and documentation. Please ensure all references are updated accordingly.
src/content/examples/cursor/Cursor.svelte (1)
1-4: LGTM! Imports are properly organized.
The necessary components are imported from the correct locations.
src/lib/maplibre/map/MapLibre.svelte (2)
100-100: LGTM!
The cursor property is correctly destructured from props.
21-21: Verify cursor implementation completeness
Let's verify the cursor implementation across the codebase.
Also applies to: 100-100, 201-204, 316-321
✅ Verification successful
Let me try a different approach to gather the necessary information since the svelte file type wasn't recognized.
Let me check the cursor example implementation to complete the verification.
Cursor implementation is properly implemented and documented
The cursor implementation is complete and well-integrated:
- The
cursorprop is properly implemented inMapLibre.sveltewith proper type definition and reactive updates - There's a dedicated example (
src/content/examples/cursor/Cursor.svelte) demonstrating various cursor styles - The implementation correctly handles cursor state changes through the canvas element
- No conflicting cursor implementations were found in the codebase
- The cursor property is properly reset when undefined
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check cursor-related implementations and documentation
# Check for cursor usage in other components
echo "Checking cursor usage in components..."
rg -l "cursor" --type svelte
# Check for potential conflicting cursor styles
echo "Checking for potential cursor style conflicts..."
rg -l "style\.cursor|\.style\.cursor" --type svelte
# Check for cursor documentation in examples
echo "Checking cursor documentation..."
rg -l "cursor" --type md
Length of output: 484
Script:
#!/bin/bash
# Search for cursor-related code in all files
echo "Checking cursor usage in all files..."
rg -l "cursor"
# Check the content of the cursor example documentation
echo -e "\nChecking cursor example content..."
cat src/content/examples/cursor/content.svelte.md
# Search for style.cursor patterns in all files
echo -e "\nChecking cursor style patterns..."
rg "style\.cursor|\.style\.cursor"
Length of output: 1742
Script:
#!/bin/bash
# Check the cursor example implementation
echo "Checking cursor example implementation..."
cat src/content/examples/cursor/Cursor.svelte
# Check the MapLibre component for cursor-related code
echo -e "\nChecking MapLibre cursor implementation..."
rg -A 5 -B 5 "cursor" src/lib/maplibre/map/MapLibre.svelte
Length of output: 2564
| onmousemove={(e) => { | ||
| cursor = e.features![0].properties.cursor; | ||
| }} | ||
| onmouseleave={(e) => { | ||
| cursor = undefined; | ||
| }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety in event handlers.
The non-null assertion on e.features! could lead to runtime errors. Consider adding proper type checking:
-onmousemove={(e) => {
- cursor = e.features![0].properties.cursor;
-}}
-onmouseleave={(e) => {
- cursor = undefined;
-}}
+onmousemove={(e) => {
+ if (e.features && e.features.length > 0) {
+ cursor = e.features[0].properties.cursor;
+ }
+}}
+onmouseleave={() => {
+ cursor = undefined;
+}}📝 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.
| onmousemove={(e) => { | |
| cursor = e.features![0].properties.cursor; | |
| }} | |
| onmouseleave={(e) => { | |
| cursor = undefined; | |
| }} | |
| onmousemove={(e) => { | |
| if (e.features && e.features.length > 0) { | |
| cursor = e.features[0].properties.cursor; | |
| } | |
| }} | |
| onmouseleave={() => { | |
| cursor = undefined; | |
| }} |
🧰 Tools
🪛 eslint
[error] 58-58: 'e' is defined but never used.
(@typescript-eslint/no-unused-vars)
| const data: GeoJSON.FeatureCollection = { | ||
| type: 'FeatureCollection', | ||
| features: CURSORS.map((cursor, i) => ({ | ||
| type: 'Feature', | ||
| geometry: { | ||
| type: 'Point', | ||
| coordinates: [ | ||
| center[0] + Math.cos((i / CURSORS.length) * Math.PI * 2) * 15, | ||
| center[1] + Math.sin((i / CURSORS.length) * Math.PI * 2) * 15 | ||
| ] | ||
| }, | ||
| properties: { | ||
| cursor | ||
| } | ||
| })) | ||
| }; |
There was a problem hiding this comment.
Fix GeoJSON type definition.
The ESLint error about undefined GeoJSON can be resolved by either:
- Adding the @types/geojson package
- Explicitly importing GeoJSON types
+import type { Feature, FeatureCollection, Point } from 'geojson';
+
-const data: GeoJSON.FeatureCollection = {
+const data: FeatureCollection = {
type: 'FeatureCollection',
- features: CURSORS.map((cursor, i) => ({
+ features: CURSORS.map((cursor, i): Feature<Point> => ({
type: 'Feature',
geometry: {
type: 'Point',📝 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.
| const data: GeoJSON.FeatureCollection = { | |
| type: 'FeatureCollection', | |
| features: CURSORS.map((cursor, i) => ({ | |
| type: 'Feature', | |
| geometry: { | |
| type: 'Point', | |
| coordinates: [ | |
| center[0] + Math.cos((i / CURSORS.length) * Math.PI * 2) * 15, | |
| center[1] + Math.sin((i / CURSORS.length) * Math.PI * 2) * 15 | |
| ] | |
| }, | |
| properties: { | |
| cursor | |
| } | |
| })) | |
| }; | |
| import type { Feature, FeatureCollection, Point } from 'geojson'; | |
| const data: FeatureCollection = { | |
| type: 'FeatureCollection', | |
| features: CURSORS.map((cursor, i): Feature<Point> => ({ | |
| type: 'Feature', | |
| geometry: { | |
| type: 'Point', | |
| coordinates: [ | |
| center[0] + Math.cos((i / CURSORS.length) * Math.PI * 2) * 15, | |
| center[1] + Math.sin((i / CURSORS.length) * Math.PI * 2) * 15 | |
| ] | |
| }, | |
| properties: { | |
| cursor | |
| } | |
| })) | |
| }; |
🧰 Tools
🪛 eslint
[error] 28-28: 'GeoJSON' is not defined.
(no-undef)
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (4)
src/routes/components/[slug]/+page.ts (1)
28-29: Add type safety for the shiki highlighterThe shiki highlighter is added to the return object without type information. Consider adding proper typing to ensure type safety.
+import type { Highlighter } from 'shiki'; return { Content: post.default, meta: { ...post.metadata, slug }, - shiki: await shiki + shiki: (await shiki) as Highlighter | null };src/content/examples/cursor/Cursor.svelte (2)
5-26: Consider improving configurability and modularity.The component could be more flexible and maintainable with these changes:
- Make
centercoordinates configurable via props to allow different map centers- Move
CURSORSarray to a separate constants file for reuse across components<script lang="ts"> import { MapLibre, GeoJSONSource } from 'svelte-maplibre-gl'; import CircleLayer from '$lib/maplibre/layers/CircleLayer.svelte'; + export let center = [0, 0]; let cursor: string | undefined = $state(); - const center = [0, 0]; - const CURSORS = [ - 'cell', - 'col-resize', - // ... other cursors - ]; + import { CURSORS } from '$lib/constants/cursors';
46-69: Enhance component configurability.Consider making the component more configurable and maintainable:
- Make the zoom level configurable via props
- Extract paint properties to a separate configuration object
<script lang="ts"> + export let initialZoom = 2; + const circlePaint = { + 'circle-radius': 12, + 'circle-color': '#007cbf', + 'circle-stroke-color': '#fff', + 'circle-stroke-width': 3 + }; </script> <MapLibre class="h-[55vh] min-h-[300px]" style="https://basemaps.cartocdn.com/gl/voyager-gl-style/style.json" - zoom={2} + zoom={initialZoom} {cursor} center={{ lng: 0, lat: 0 }} > <GeoJSONSource {data}> <CircleLayer {/* ... event handlers ... */} - paint={{ - 'circle-radius': 12, - 'circle-color': '#007cbf', - 'circle-stroke-color': '#fff', - 'circle-stroke-width': 3 - }} + paint={circlePaint} /> </GeoJSONSource> </MapLibre>src/lib/maplibre/map/MapLibre.svelte (1)
201-203: Optimize initial cursor setupThe current implementation has redundant null checks and duplicates logic with the reactive updates.
Consider simplifying to:
- if (cursor) { - map.getCanvas().style.cursor = cursor ?? ''; - } + map.getCanvas().style.cursor = cursor ?? '';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
README.md(2 hunks)src/content/examples/cursor/Cursor.svelte(1 hunks)src/content/examples/cursor/content.svelte.md(1 hunks)src/lib/maplibre/map/MapLibre.svelte(4 hunks)src/routes/components/[slug]/+page.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- src/content/examples/cursor/content.svelte.md
🧰 Additional context used
🪛 eslint
src/content/examples/cursor/Cursor.svelte
[error] 28-28: 'GeoJSON' is not defined.
(no-undef)
🔇 Additional comments (7)
src/routes/components/[slug]/+page.ts (3)
3-7: LGTM: Imports are well-structured
The imports are properly organized, with clear separation between app-specific and third-party dependencies.
Line range hint 19-24: Verify the impact of path changes
The import path has changed from 'examples' to 'components'. This could potentially break existing documentation.
Let's verify the existence of content in both locations:
9-13: Consider adding error handling for highlighter initialization
While the conditional engine selection is good for browser compatibility, consider:
- Adding error handling for WASM import failures
- Moving the initialization into the load function to avoid potential SSR issues
Let's verify if there are any existing error handlers for WASM imports:
Consider this safer implementation:
-const shiki = createHighlighter({
- themes: [dark],
- langs: [svelte],
- engine: browser ? createOnigurumaEngine(import('shiki/wasm')) : createJavaScriptRegexEngine()
-});
+async function createSafeHighlighter() {
+ try {
+ return await createHighlighter({
+ themes: [dark],
+ langs: [svelte],
+ engine: browser ? createOnigurumaEngine(import('shiki/wasm')) : createJavaScriptRegexEngine()
+ });
+ } catch (error) {
+ console.error('Failed to initialize syntax highlighter:', error);
+ return null;
+ }
+}src/content/examples/cursor/Cursor.svelte (2)
28-43:
Fix GeoJSON type definition.
The GeoJSON type is still undefined. This needs to be fixed for proper type safety.
+import type { Feature, FeatureCollection, Point } from 'geojson';
+
-const data: GeoJSON.FeatureCollection = {
+const data: FeatureCollection = {
type: 'FeatureCollection',
- features: CURSORS.map((cursor, i) => ({
+ features: CURSORS.map((cursor, i): Feature<Point> => ({
type: 'Feature',
geometry: {
type: 'Point',🧰 Tools
🪛 eslint
[error] 28-28: 'GeoJSON' is not defined.
(no-undef)
55-60:
Improve type safety in event handlers.
The non-null assertion on e.features! should be replaced with proper type checking.
-onmousemove={(e) => {
- cursor = e.features![0].properties.cursor;
-}}
-onmouseleave={() => {
- cursor = undefined;
-}}
+onmousemove={(e) => {
+ if (e.features && e.features.length > 0) {
+ cursor = e.features[0].properties.cursor;
+ }
+}}
+onmouseleave={() => {
+ cursor = undefined;
+}}src/lib/maplibre/map/MapLibre.svelte (2)
21-21: LGTM: Props interface change
The cursor property is correctly defined as an optional string type, following MapLibre GL JS API patterns.
316-321: Verify cursor cleanup and usage patterns
While the implementation looks functional, let's verify a few aspects:
✅ Verification successful
Cursor implementation is properly scoped and handled
The cursor implementation is well-contained within the MapLibre component with no conflicting usage across the codebase:
- The cursor style manipulation is only present in MapLibre.svelte
- The cursor prop is properly typed and exported
- The effect correctly updates the cursor style when the cursor prop changes
- No cleanup is needed as Svelte's reactivity will handle the component destruction automatically, and the cursor style will be cleaned up with the canvas element
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check cursor-related patterns in the codebase
# Check for other cursor style manipulations that might conflict
rg -l "style\.cursor" --type svelte
# Check for cursor-related cleanup in destroy/cleanup blocks
rg -l "cursor.*destroy|destroy.*cursor" --type svelte
# Check for example usage patterns
rg -l "<MapLibre.*cursor.*>" --type svelte
Length of output: 231
Script:
#!/bin/bash
# Description: Check cursor-related patterns in the codebase without file type filtering
# Check for other cursor style manipulations that might conflict
rg -l "style\.cursor"
# Check for cursor-related cleanup in destroy/cleanup blocks
rg -l "cursor.*destroy|destroy.*cursor"
# Check for example usage patterns
rg -l "<MapLibre.*cursor"
# Check for cursor prop usage in MapLibre component
rg -l "export let cursor" src/lib/maplibre/map/MapLibre.svelte
# Look for other cursor-related code in MapLibre component
rg "cursor" src/lib/maplibre/map/MapLibre.svelte
Length of output: 395
| $effect(() => { | ||
| cursor; | ||
| if (map && !firstRun) { | ||
| map.getCanvas().style.cursor = cursor ?? ''; | ||
| } | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize reactive cursor updates
The current implementation can be improved for better maintainability and efficiency.
Consider combining initial setup and reactive updates into a single effect:
- $effect(() => {
- cursor;
- if (map && !firstRun) {
- map.getCanvas().style.cursor = cursor ?? '';
- }
- });
+ $effect(() => {
+ if (map) {
+ map.getCanvas().style.cursor = cursor ?? '';
+ }
+ });This change:
- Removes unnecessary cursor dependency tracking
- Eliminates the need for firstRun check since the cursor should be set whenever it changes
- Combines initial setup and reactive updates into a single location
📝 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.
| $effect(() => { | |
| cursor; | |
| if (map && !firstRun) { | |
| map.getCanvas().style.cursor = cursor ?? ''; | |
| } | |
| }); | |
| $effect(() => { | |
| if (map) { | |
| map.getCanvas().style.cursor = cursor ?? ''; | |
| } | |
| }); |
<MapLibre {cursor} />Summary by CodeRabbit
Release Notes
New Features
Cursorcomponent for interactive cursor style changes on the map.cursorproperty to theMapLibrecomponent, allowing custom cursor styles.Bug Fixes
Documentation