Skip to content
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

feat: Primitive array control #49

Merged
merged 6 commits into from
Mar 20, 2024
Merged

feat: Primitive array control #49

merged 6 commits into from
Mar 20, 2024

Conversation

TrangPham
Copy link
Contributor

Screen.Recording.2024-03-19.at.3.11.50.PM.mov

Comment on lines 68 to 76
// Note: For primative arrays, ArrayControlProps.data is an array
// For object arrays, ArrayLayoutProps.data is a number
const dataSource: any[] =
typeof data === "number"
? range(data)
: // antd List component doesn't like undefined/null in the dataSource
data?.map((item: any) => item ?? "")
const dataLength = typeof data === "number" ? data : data?.length || 0

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the main difference for the Object Array vs the Primitive Array

Comment on lines 1 to 5
/* eslint-disable @typescript-eslint/no-explicit-any */
/* eslint-disable @typescript-eslint/no-unsafe-call */
/* eslint-disable @typescript-eslint/no-unsafe-member-access */
/* eslint-disable @typescript-eslint/no-unsafe-return */
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a lot of eslint things to disable--did you intend to leave these in?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since json-forms has a lot of any types these errors are showing up. Maybe if I'm able to split the component it might be slightly better. Any suggestions?

@DrewHoo DrewHoo changed the title Primative array control Primitive array control Mar 20, 2024
@TrangPham TrangPham changed the title Primitive array control feat: Primitive array control Mar 20, 2024
@@ -58,6 +65,15 @@ export function ObjectArrayControl({
const options: ArrayControlOptions =
(uischema.options as ArrayControlOptions) ?? {}

// Note: For primative arrays, ArrayControlProps.data is an array
// For object arrays, ArrayLayoutProps.data is a number
const dataSource: any[] =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you try unknown instrad of any here? Any is pretty dangerous and can typically be replaced by unknown without too much work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using unknown did not do the trick

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After playing around with it a bit more got it to work

@@ -28,7 +35,7 @@ export function ObjectArrayControl({
rootSchema,
uischemas,
required,
}: ArrayLayoutProps) {
}: ArrayLayoutProps | ArrayControlProps) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a smell to me--have you tried splitting this into two components that wrap ancomponent with common logic? Ie create the datasource array in the higher-level components and pass to the shared component?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try that

Copy link
Contributor

@DrewHoo DrewHoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@TrangPham TrangPham merged commit b8394e0 into main Mar 20, 2024
2 of 3 checks passed
@TrangPham TrangPham deleted the primative-array-control branch March 20, 2024 23:13
Copy link

🎉 This PR is included in version 1.9.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants