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

Object formatting #34

Closed
jkk4999 opened this issue Feb 25, 2024 · 17 comments
Closed

Object formatting #34

jkk4999 opened this issue Feb 25, 2024 · 17 comments
Assignees

Comments

@jkk4999
Copy link

jkk4999 commented Feb 25, 2024

Hi,

When adding an object, the closing parentheses are indented. Thanks for your contribution!

Screenshot 2024-02-25 at 9 59 43 AM
@CarlosNZ
Copy link
Owner

Hi, that's odd. Can you provide more details of the steps you took to get this? And also, what version of the component are you using (if not the latest, try upgrading to 1.5.1)?

It looks correct if I just type that info straight into the demo system:
Screenshot 2024-02-26 at 4 19 53 PM

@jkk4999
Copy link
Author

jkk4999 commented Feb 26, 2024

I'm using 1.5.1. and React 18.2.0. This minimal code produces the issue.

return (


<JsonEditor
data={data}
restrictEdit={({ level }) => level === 0}
onUpdate={({ newData }) => {
console.log('new flowConfig data is');
console.dir(newData)
setData(newData)
Screenshot 2024-02-26 at 6 56 06 AM

        }} />
  </div>

)

@CarlosNZ
Copy link
Owner

Okay, I've been able to reproduce this: https://codesandbox.io/p/sandbox/wizardly-yalow-g5qlym?file=%2Fsrc%2Fstyles.css%3A3%2C18

The problem is due to a text-align: center CSS on an ancestor element, which is cascading down into this component. You can fix it yourself by explicitly setting text-align: left on the immediate parent of json-edit-react, but I'm going to do a quick fix and force this on the component itself, so you can upgrade later on and have it fixed for you :)

@CarlosNZ
Copy link
Owner

CarlosNZ commented Feb 27, 2024

Righto, v1.5.3 out now. Please let me know if it fixes your case and I'll close the issue.

Thanks for bringing it to my attention

@CarlosNZ CarlosNZ self-assigned this Feb 27, 2024
@jkk4999
Copy link
Author

jkk4999 commented Feb 27, 2024

1.5.3 fixed the issue...much appreciated! In my particular use case, I'm using your component to create metadata. When adding a new top-level key, I always want the value to be of type 'object'. Currently, it defaults to be type 'string' and then I have to change it to 'object'. When adding child keys, I want to remove the 'object' type. Any guidance would be appreciated.

Screenshot 2024-02-27 at 7 31 57 AM

@CarlosNZ
Copy link
Owner

When adding a new top-level key, I always want the value to be of type 'object'. Currently, it defaults to be type 'string' and then I have to change it to 'object'.

You can set the default value prop: defaultValue={{}} (or defaultValue={{myKey: "defaultValue"}}). But this will apply to all new elements, not just the top level one, which is not what you want, right?

I guess I could make the defaultValue prop also take a function to determine what it should be based on data state -- would that be useful?

When adding child keys, I want to remove the 'object' type. Any guidance would be appreciated.

You can restrict the type selection list: restrictTypeSelection={['string', 'boolean', 'number', 'null']}. But again, I suspect you want it to be different for the top level. So a dynamic default value is probably the answer.

Thanks for the suggestion. I've made an issue to address this: #35

It should be fairly straightforward, I'll try and update it in the next couple of days.

@CarlosNZ
Copy link
Owner

Original issue solved

@jkk4999
Copy link
Author

jkk4999 commented Feb 27, 2024

Yes, adding the ability for the default value and restrictTypeSelection props to take a function would address both issues. Appreciate your support!

@CarlosNZ
Copy link
Owner

Yes, adding the ability for the default value and restrictTypeSelection props to take a function would address both issues. Appreciate your support!

restrictTypeSelection already can take a function, so it's just the default value. I'll get on to that soon.

@CarlosNZ
Copy link
Owner

CarlosNZ commented Feb 28, 2024

Okay, @jkk4999, you can try v1.6.0 now, with your requested feature.

You should be able to achieve what you want by specifying:

// Create empty objects at the root level, strings elsewhere:
 defaultValue={({ level }) => {
      if (level === 0) return {}
      return 'New Value'
    }},
// Prevent changing values to objects or arrays:
restrictTypeSelection={['string', 'number', 'boolean', 'null']}

There's an example of something similar in the Demo -- if you select "Client List" data set and add a new item at the root level, you'll get a pre-filled structured object, but just plain strings elsewhere.

Thanks for the suggestion -- not sure why I hadn't done that already -- you could already specify functions for pretty much all other values. ;)

@jkk4999
Copy link
Author

jkk4999 commented Feb 28, 2024

That works...thanks! In my use case, I need to restrict the values of child nodes to 'string', 'number' or 'boolean'. To that end, I make use of the onEdit function and inspect the value of newValue prop. Per your documentation, if you return false or a string, the internal data state won't actually be updated. However, it seems like it is. If I enter a value that doesn't pass my validation rule I'm returning a string. I see the error message but the control displays the entered value. My expectation would be the control discards the entered value and displays the previous value in state.

In a related issue, the error message persists for a few seconds. It would be preferable that the error message is displayed until such time as the error is resolved.

For reference, here is the code:

onEdit={({ newData, currentData, newValue, currentValue, name, path }) => {
console.log('onEdit props are');
console.dir({
newData: newData,
currentData: currentData,
newValue: newValue,
currentValue: currentValue,
name: name,
path: path
})

           if (path.length > 0) {
              // editing child node
              //@ts-ignore
              if (newValue === 'string' || newValue === 'number' || newValue === 'boolean') {
                 console.log('dispatching json change')
                 dispatch(setSelectedObjFlowConfig(newData))
                 saveFlowPreferences(newData);
              }
              else {
                 return 'incorrect value';
              }
           }
        }}

@CarlosNZ
Copy link
Owner

CarlosNZ commented Feb 28, 2024

Per your documentation, if you return false or a string, the internal data state won't actually be updated. However, it seems like it is

Oh, that's a good point. I should be able to change this pretty easily to only update the internal state if the update function doesn't return an error.

The only tricky thing is that the update function can take a significant amount of time (if it's updating to an API), but I want the UI update to be instant. I guess we could have the UI update, but then switch back to the original value if the update function subsequently fails (and shows the error). Would that be okay you think?

@CarlosNZ
Copy link
Owner

In my use case, I need to restrict the values of child nodes to 'string', 'number' or 'boolean'. To that end, I make use of the onEdit function and inspect the value of newValue prop.

Is restricting the available types in restrictTypeSelection not enough?

@CarlosNZ
Copy link
Owner

In a related issue, the error message persists for a few seconds. It would be preferable that the error message is displayed until such time as the error is resolved.

I'll have to think about this/play around with it. In light of your other comment, if we don't actually update the internal state on error, then it won't actually be in an illegal state. This is how it works for other errors, such as invalid JSON -- the internal state never gets updated, so the error message just shows for a certain time.

@jkk4999
Copy link
Author

jkk4999 commented Feb 28, 2024 via email

@CarlosNZ
Copy link
Owner

CarlosNZ commented Mar 1, 2024

Hi @jkk4999 , just to let you know that I've published v1.6.1 which has a fix for #37. If your update function returns false or a string value, the internal data won't be updated and the error will show.

Hope that works for you.

@jkk4999
Copy link
Author

jkk4999 commented Mar 1, 2024

Perfect. Thanks so much for your efforts!

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

No branches or pull requests

2 participants