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: autocomplete subprops and allow string values for readOnly in resolveData #625

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mkilpatrick
Copy link
Contributor

This updates the TS types of resolveData such that:

readOnly can now return a boolean OR a string. Supporting string is helpful when you have a 'custom' field defined for a level which contains multiple fields underneath, rather than using objectFields. resolveData can pass a string value of the subfield being set for readOnly.

As an example, given this type:

type MyProps = {
  foo: {
    bar: {
      id: string;
      value: string;
  }
}

and I want to create a custom Puck field at the bar level, I have id and value as subprops. resolveData works at the level of bar, so if you wanted to set value as readOnly, that is not possible with

resolveData: () => {
  return {
    readOnly: { 'foo.bar': true }
  }
}

we do this instead now

resolveData: () => {
  return {
    readOnly: { 'foo.bar': 'value' }
  }
}

and then in the field's render function the readOnly prop would have a value of 'value'.

The readOnly acceptable values now autocomplete to any valid subfield. Prior to this change only the top-level fields were enumerated by the type. Given the MyProps example above:

resolveData: () => {
  return {
    readOnly: { 'HERE': 'value' } 
  }
}

HERE would autocomplete as: 'foo'

Now it autocompletes as : 'foo' | 'foo.bar' | 'foo.bar.id' | 'foo.bar.value'

More context around these changes can be found here.

Copy link

vercel bot commented Sep 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
puck-docs ❌ Failed (Inspect) Sep 24, 2024 10:39pm

Copy link

vercel bot commented Sep 24, 2024

@mkilpatrick is attempting to deploy a commit to the Measured Team on Vercel.

A member of the Team first needs to authorize it.

@mkilpatrick
Copy link
Contributor Author

The DotBranch type could be used in a number of more places.

Copy link
Member

@chrisvxd chrisvxd left a comment

Choose a reason for hiding this comment

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

I like the DotBranch type here (and have been meaning to do this for a while) but I'm unsure about the readOnly string solution.

and I want to create a custom Puck field at the bar level, I have id and value as subprops. resolveData works at the level of bar, so if you wanted to set value as readOnly, that is not possible with

resolveData: () => {
  return {
    readOnly: { 'foo.bar': true }
  }
}

This is true, but I would expect this to be the appropriate API -

resolveData: () => {
  return {
    readOnly: { 'foo.bar.value': true }
  }
}

The challenge is then that the custom field won't have any way to receive the readOnly value. Rather than make the value a string, it might be cleaner to introduce a new prop to the custom field type that provides the readOnly object

render: ({ readOnly, readOnlyData }) => {
  
  // false
  console.log(readOnly);
  
  // { 'foo.bar.value': true }
  console.log(readOnlyData);
  
  return <div />
}

Whilst we're at it, maybe we should consider that resolveData method should return a nested object rather than dot-notated fields. I can't remember the reason we didn't do this in the first place.

resolveData: () => {
  return {
    readOnly: { 
      foo: {
        bar: {
          value: true
        }
      }
    }
  }
}

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

Successfully merging this pull request may close these issues.

2 participants