json-forms icon indicating copy to clipboard operation
json-forms copied to clipboard

schema.enum enhancement for issue #85 and #22

Open hxh-robb opened this issue 7 years ago • 6 comments

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

hxh-robb avatar Jan 15 '18 02:01 hxh-robb

Ohh, I have made this in my branch because of some projects needs i am working on! (not in the pull request initialDate) #113. Maybe we can merge our work... My implementation seems much more simpler than yours at first glance. I have not commit my change yet to github because i have not tested it as much as i will like to, but as far as i tested it, it works. My solution is supposed to work the way the original brutusin works (just an array within enum) and ours, depending on the schema (Key,Value). Does your improvement work with the bootstrap-select Brutusin let us to enchance? (http://silviomoreto.github.io/bootstrap-select/examples/) I think it is not possible.

juamar avatar Mar 05 '18 23:03 juamar

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 appendedProperties is just because I don't want to change the original rendering logic too much(Following the Open-Closed Principle). With the new option, I can work with it in my own renderAppendedProperties function without touch the original one, so that if something go wrong in my code it won't hurt the original function.

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.

hxh-robb avatar Mar 06 '18 01:03 hxh-robb

At night (Madrid Time) i will try to test it a little bit more with a JSFiddle and commit my update in any case.

juamar avatar Mar 06 '18 12:03 juamar

All done. My fork: https://github.com/juamar/json-forms/tree/enumKeyValue Some testing: http://plainsite.tecandweb.net/brutusin/ (I wasn't able to make fiddle to work since i don't have some CDN at hand nor a testing site with HTTPS).

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).

juamar avatar Mar 06 '18 23:03 juamar

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 appendedProperties feature in your fork, right?

hxh-robb avatar Mar 07 '18 02:03 hxh-robb

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?

juamar avatar Mar 07 '18 09:03 juamar