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

Add support to optional "select" fields #783

Closed
nickfla1-mxm opened this issue Jan 10, 2025 · 6 comments
Closed

Add support to optional "select" fields #783

nickfla1-mxm opened this issue Jan 10, 2025 · 6 comments
Assignees

Comments

@nickfla1-mxm
Copy link

Currently the select field only support String, Number or Boolean types but in the case of optional fields forcing the user to select a value can be counter-intuitive.

Given the following example:

fields: {
  mySelect: {
    type: 'select',
    options: [
      { label: 'A', value: 'a' },
      { label: 'B', value: 'b' },
    ]
  }
}

if mySelect represents an optional value we could either

fields: {
  mySelect: {
    type: 'select',
    options: [
      { label: '', value: undefined }, // allow having `undefined` or `null` values
      { label: 'A', value: 'a' },
      { label: 'B', value: 'b' },
    ]
  }
}

or (cleaner in my opinion)

fields: {
  mySelect: {
    type: 'select',
    optional: true, // required: false alternatively
    options: [
      { label: 'A', value: 'a' },
      { label: 'B', value: 'b' },
    ]
  }
}

I am available to contribute directly to this feature if approved!

@FedericoBonel
Copy link
Collaborator

Hello @nickfla1-mxm,

I think adding an optional parameter could work, but it might also be useful to support other types too, such as objects. Perhaps we could combine both approaches to make it more flexible?

@nickfla1-mxm
Copy link
Author

I agree, selects could have any data type as a value, though you could achieve the same with a string/numeric index an a mapping on the component side, the value stored in the data wouldn't be the exact value

@chrisvxd
Copy link
Member

Hm does the following not already work? Puck should respect the optional typing.

type Props = {
  MyComponent: {
    fruit?: 'apple' | 'banana' // Make optional with `?`
  }
}

const conf: Config<Props> = {
  components: {
    MyComponent: {
      fields: {
        fruit: {
          type: "select",
          options: [
            { label: '', value: undefined }, // Should work because `fruit` is optional
            { label: 'Apple', value: 'apple' },
            { label: 'Banana', value: 'banana' },
          ]
        }
      }
    }
  }
}

If not, this is likely a bug, rather than requiring a new feature

@nickfla1-mxm
Copy link
Author

At the time I've opened this issue you couldn't set value to undefined even if optional in the component props, I haven't tried on the latest Puck version

@chrisvxd
Copy link
Member

Okay. I don't remember fixing this so I'm going to assume it's a bug and mark this as ready.

@chrisvxd
Copy link
Member

Closing in favour of bug ticket here: #895

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

No branches or pull requests

3 participants