-
Notifications
You must be signed in to change notification settings - Fork 167
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
schema.enum enhancement for issue #85 and #22 #114
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New feature "appendedProperties" for issue #22
The following link is the demo of this feature
https://jsfiddle.net/hxh_robb/xk663z01/
Ohh, I have made this in my branch because of some projects needs i am working on! (not in the pull request initialDate) #113. |
Hi juamar, can you post a demo of yours, sounds good that yours is simpler, which means it would be more easier to use. The reason I made a new schema option And you're right, my code doesn't work with the bootstrap-select. If there is a multiple select field, things might go wrong, but I'm not sure, I'll test it later. BTW does your version finish this work? it would be great to see this enhancement. |
At night (Madrid Time) i will try to test it a little bit more with a JSFiddle and commit my update in any case. |
All done. My fork: https://github.com/juamar/json-forms/tree/enumKeyValue You are right, i am not taking in account the open-closed principle because my editions are within the original implementation. I suppose further testing will be good in my code, but i am confident my new code will not compromised other functionality from brutusin. Muy changes affects only the enum rendering block. Meanwhile testing, I saw that I was using JQuery to select the option from value. Now I don't. Just in case, in the example, the form on the left uses a Dictionary (key-value) as enum, and the one on the right uses an array (as the original implementation, showing that both ways are compatible in this new code). |
Hi juamar, I've had a look at your demo, seems that yours is much more clear and concise. I'll check out your brunch and see how to merge it into mine. BTW, you didn't implement the equivalent |
No, i don't... But we got a situation here... I think we are not taking in account this: https://github.com/brutusin/json-forms/blob/master/README.md#status I mean if @idelvall will not take care of this repo any more, who knows when our pull request will be taken in consideration. I am not interested in appendedProperties functionality right now, so, i will correct my self, i am only solving issue #85 . Maybe you can add it. The question is... Where to make the pull request? Where to continue this project? |
Use decorator to meet this requirement is not intuitive, so I made enum rendering accept a {text:XXX, value:XXX} object to render display text and value respectively