-
Notifications
You must be signed in to change notification settings - Fork 46
Feature/35514 csv import mapping #506
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
base: main
Are you sure you want to change the base?
Conversation
c1c2eec to
249df9f
Compare
|
/cursor review |
77b55c7 to
c7a2d80
Compare
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.
Pull request overview
This PR implements CSV import field mapping with automatic type inference for the Framer plugin. It replaces complex state management with a custom minirouter solution and updates the framer-plugin dependency from beta to stable version.
Key Changes:
- Added field mapping interface with automatic type inference for CSV columns
- Implemented a lightweight routing system (minirouter) to manage plugin views
- Refactored CSV import logic into modular utilities for better maintainability
Reviewed changes
Copilot reviewed 17 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Updated framer-plugin from 3.9.0-beta.1 to stable 3.9.0 |
| plugins/csv-import/src/utils/typeInference.ts | New utility for inferring field types from CSV data |
| plugins/csv-import/src/utils/prepareImportPayload.ts | Refactored import payload preparation with field mapping support |
| plugins/csv-import/src/utils/parseCSV.ts | Extracted CSV parsing logic into dedicated module |
| plugins/csv-import/src/utils/importCSV.ts | Separated import execution logic |
| plugins/csv-import/src/utils/fieldReconciliation.ts | Handles field creation and removal operations |
| plugins/csv-import/src/utils/assert.ts | Custom assertion utility |
| plugins/csv-import/src/routes/FieldMapper.tsx | New field mapping UI component with type compatibility checking |
| plugins/csv-import/src/routes/Home.tsx | Home route component |
| plugins/csv-import/src/routes/ManageConflicts.tsx | Updated to use new import types |
| plugins/csv-import/src/minirouter.tsx | Custom routing implementation for plugin views |
| plugins/csv-import/src/main.tsx | Wrapped app with MiniRouterProvider |
| plugins/csv-import/src/App.tsx | Refactored to use routing and new import flow |
| plugins/csv-import/src/App.css | Extensive styling additions for field mapper |
| plugins/csv-import/src/components/SelectCSVFile.tsx | Extracted file selection component |
| plugins/csv-import/src/components/CollectionSelector.tsx | Minor formatting improvements |
| plugins/csv-import/src/components/*.tsx | New icon components |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Comment @cursor review or bugbot run to trigger another review on this PR
| @@ -0,0 +1,75 @@ | |||
| import { type Collection, framer, type UIOptions } from "framer-plugin" | |||
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.
home-rolled minirouter. Makes it much easier to navigate between views in an explicit way while also passing in callbacks to treat views as asynchronous information collection with a promise (see app.tsx)
0501880 to
5953cf4
Compare
5953cf4 to
fcd922a
Compare
|
|
||
| // Add new fields using collection.addFields() | ||
| if (fieldsToAdd.length > 0) { | ||
| await collection.addFields(fieldsToAdd) |
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.
I can't figure out which isAllowedTo key to use here, it seems to be checked in App.tsx but still I'm seeing:
Invoking protected message type "addCollectionFields2" without checking permissions first
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.
Should be Collection.addFields
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.
Likely bug in https://github.com/framer/FramerStudio/blob/master/src/plugin-api/src/permissions.ts#L414-L415
should be addCollectionFields2
| } | ||
| } | ||
|
|
||
| export async function createNewFieldsInCms(collection: Collection, mappings: FieldMappingItem[]): Promise<void> { |
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.
Possible bug here, but may be in vekter: https://github.com/framer/company/issues/35575#issuecomment-3671377441
| @@ -0,0 +1,689 @@ | |||
| import type { Collection, Field } from "framer-plugin" | |||
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.
Let's bring the UI design of this in line with Notion as much as possible
Closes framer/company#35514
Description
minirouterand more procedural workflowsTest files:
jobs.csv
people_added_column.csv
people_duplicate.csv
people_empty.csv
people_missing_slug.csv
people_removed_column.csv
people_reordered.csv
people.csv
people2_changed_column_type.csv
people2_draft.csv
people2_renamed_column.csv
people2_withjob.csv
Testing
Useful synthetic test imports: