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
Option for editing of undefined non-required fields (bug with tables of objects) #980
base: master
Are you sure you want to change the base?
Conversation
As developer I want to let user edit (set, update) optional fields if future even if inital value was empty. Reproduce: 1) User creates new object A. 2) User sets only mandatory fields and keep optional fields blank. 3) User saves object. 4) User decided to set optional filed of A. He starts editing orbject A Current behavior: 5) Json-editor is not displaying editor for optional filed beacuse it is not initial object creation and optional field value is undefined. Expected behaviour: 6) Json-editor displayes editor with empty value for optional field.
The live example: my object reprecents a day of the library schedule:
Initially library is closed on Monday, so all fields except DayCode are undefined. But what if user want to change schedule and set startTime and endTime for Modnay? Without extra option user will see just dayCode editor and no one more editors. |
Could you add an example and/or screenshot, I don't quite understand. |
The reason of missing fields in the firest image is that server returned the next array:
just removed extra editors. But it is not what I'd want. |
Did you tried |
Yes, I've tried to apply it for field, object and array - no effect.
|
@Krivich Please post a usable schema or even better a link. |
@schmunk42 may I send schema to your email in profile? The project is not yet released so I'd want to limit access for a while |
The |
I've also tried this, without any luck. Thanks. |
The fields are readonly, is that the problem? |
Sorry, not clear why they are readonly. Did I use any market for that? |
We need some tests for the use-cases first. Test-PRs welcome (only tests with |
This is the corrected schema:
In the current latest release there is a bug that leaves the optional editors disabled. I fixed this bug already but a release is necessary. @Krivich : you could try this schema with the master version of json-editor |
I appreciate if you hint how to do that. Currently json-editor is loaded by npm inside docker build like:
Is there any option to make it get version associated with master? |
I've referenced the latest master version in my npm this way:
If I add
|
This code is resposible for disabled editors if I show them by
|
i meant like this: const editor = new JSONEditor(element, {
|
@germanbisurgi, the side-effect is checkbox near every optional field: |
Yeah, I noticed that also. |
The additional checkboxes is an expected behaviour. That is how the json-editor allows you to opt_in properties |
Interesting observation: editors of empty optional filed of the childern object of the root object are displayed, all missing editors are in the object which bound to the root over the array. In other words
I'm not familiar with the test library so just trying to implement some realistic but experiencing lack of knowledges. Sorry. |
UPDATED: sorry you are right. The field in the root is displayed becaue of empty string. Below is demostration.
|
The easy way to fix side-efect of
But it brings the next problem - fields which was empty are disabled and without checkbox it is impossible to enable them. So it seems that my proposal in MR solves the problem cleaner - editors are visible and enabled. Please tell me what I need to do to satisfy MR merge criterias? |
Optional "empty" properties is one the reasons why json-editor uses checkboxes. |
I might upderstand you incorrectly. Json-editor will automaically remove from json output optional properties which has empty value. It means that if I suppose that user may want to set optional filed later - I can safely show him all editors and json-editor will output this field only if user has set the value. What is the reason to make user check checkox to start editing empty optional field in this scenario? |
Not quite. It could be that you want to have a property that is empty, but that also has to appear in the output. How do you determine if is active or not? The answer is, you do not base it state based on its value but something else, like a checkbox. |
Sounds like very exotic case. I'd remove this field from json output untill user touch the field. If touched - output empty string. If initial value is empty string - output empty string. To let remove such "empty" value - I'd show "clear" button. It is just a raw idea. The current UX where I have to show checkbox near all optional fileds if I want to let user edit optional field later is very "heavy", I'm not really sure that non-technical user will like such UI. What do you think about my MR? Is there any chance to merge it? At least is it a good alternative, 100% non-breakin and 100% backward compatible. |
@schmunk42, @germanbisurgi what should I do to help to accept my pull-request? |
@germanbisurgi you took a closer look, can we merge this? |
This PR will also improve consistancy of json-editor because table format of array allows editing of optional fields any time, but tabs formt - not. |
Sorry, I would not merge this. |
What should I improve to make it acceptable? |
If it don't change the behaviour of show_opt_in directly it' ok for me. In particular the empty fields should not be activated automatically unless it was told to do so with an extra option setting. If you can expand your teste to prove this we can merge the PR . |
Done.
Please let me know if anything else is required from my side. |
Perfect :) @schmunk42: what do you think? |
While the PR looks good, just a README entry about the new option would is missing, I still do not fully understand it. Actually showing non required editors should be the default, unless So why do we need a new option? |
Let me prepare a short demo for you. |
As you can see table representation lets edit values any time. It means that If you want to change work hours for Monday - no problems, just input new values. Unfortunately table layout is not mobile-friendly. Once you migrate to tabs-top format which is much more mobile-friendly - you are loosing opportunity to edit work hours anytime you want. The only existing workaround for now - turn on |
Oh, maybe i know where the problem lays but i need some time to test it. I will update this post asap. Edit:
|
I think I understand now :) It's indeed not intuitive, but just how json-schema works. Acutally I think showing empty fields should be accomplished just by using But I also noticed that this does not show the fields, when data is loaded without the corresponding keys. We had such issues before, looks trivial first, but get compley. That's why I am still a bit hesitant to merge. |
As developer I want to let user edit (set, update) optional fields if future even if inital value was empty.
Reproduce:
Current behavior:
Expected behaviour: