Create spatial col fix #2385
Create spatial col fix #2385
Conversation
ConsoleProject ID: Sites (2)
Note You can use Avatars API to generate QR code for any text or URLs. |
WalkthroughAdds an editing?: boolean prop to line.svelte, point.svelte, and polygon.svelte, with onMount logic to initialize state when not editing: reset savedDefault using getDefaultSpatialData for the respective geometry, set defaultChecked to false, and clear $required. Updates prop destructuring to include editing (defaulting to false where noted). In spreadsheet.svelte, adds a null guard for spatial cells so only non-null spatial values are stringified. In store.ts, adjusts getDefaultSpatialData for polygon to return a single ring instead of two. No other public API changes beyond the new editing prop in the three components. Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (1)
85-94: Fix polygon point deletion: preserve ring closure and min verticesCurrent logic removes the closing coord and can leave an open/invalid ring. Delete the last non‑closing point and re‑close the ring; also guard the minimum (3 distinct points + closing).
- function deleteCoordinate(ringIndex: number) { - const ring = data.default?.at(ringIndex); - - ring.splice(ring.length - 1, 1); - if (ring.length === 0) { - data.default.splice(ringIndex, 1); - } - - data.default = [...(data.default || [])]; - } + function deleteCoordinate(ringIndex: number) { + const ring = data.default?.[ringIndex]; + if (!ring) return; + // Keep at least 3 distinct points (+ closing point) + if (ring.length <= 4) return; + // Remove last non-closing point, then re-close the ring + ring.splice(ring.length - 2, 1); + ring[ring.length - 1] = [...ring[0]]; + data.default = [...(data.default || [])]; + }
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte (1)
922-926: Guard undefined spatial values to avoid blank cellsJSON.stringify(undefined) yields undefined (no output). Use a nullish check so undefined falls through to the generic renderer that shows a NULL badge.
- {:else if isSpatialType(rowColumn) && row[columnId] !== null} + {:else if isSpatialType(rowColumn) && row[columnId] != null}src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
49-50: Default editing=false for consistency with line/polygonSet a default to match the other components and avoid treating an explicit false differently from undefined.
- let { data = { required: false, default: null }, editing }: Props = $props(); + let { data = { required: false, default: null }, editing = false }: Props = $props();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte(2 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/spreadsheet.svelte(1 hunks)src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts(0 hunks)
💤 Files with no reviewable changes (1)
- src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/store.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (4)
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/line.svelte (2)
46-51: New editing prop looks good; ensure parent wiringProp addition and default value are sensible. Verify callers pass editing={true} for the update flow; default false will correctly treat “create” flow.
96-102: Create-mode init is correctResetting savedDefault/defaultChecked/$required only when not editing prevents clobbering existing column configs.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/point.svelte (1)
86-92: Create-mode init matches the patternSame behavior as line/polygon; good consistency.
src/routes/(console)/project-[region]-[project]/databases/database-[database]/table-[table]/columns/polygon.svelte (1)
107-113: Create-mode init is correctMirrors line/point and avoids overwriting when editing.
There was a problem hiding this comment.
we might have to inline all these 3 into one component someday for reuse, the logic looks mostly the same to me at JS side atleast!
There was a problem hiding this comment.
I started with this approach only but a lot of mess due to the data structure and functions underlying -> leading to handling of the patterns similar to three files
What does this PR do?
Screencast.from.2025-09-18.16-21-04.webm
Test Plan
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)
Related PRs and Issues
(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
Have you read the Contributing Guidelines on issues?
(Write your answer here.)