search

preaction / Yancy

The Best Web Framework Deserves the Best Content Management System
http://preaction.me/yancy/
Other
54 stars 21 forks source link

Add array support to the UI #72

Closed wbazant closed 4 years ago

wbazant commented 4 years ago

It seems to work - initial state makes it to the screen, updates on changes, submits the right thing. It could be really inefficient or against best practices etc. as I was learning VueJS as I went along: do review.

The layout can probably be improved. The code works on arrays of arrays etc. but the layout breaks down.

Another line of further work could be to add support for objects once someone has a need for it - the recursion could work the same.

Some screenshots: https://imgur.com/a/dfYfWoj

coveralls commented 4 years ago

Coverage Status

Coverage remained the same at 93.164% when pulling 71bfc4b6b6848c3e3ef980867b76dac3b5c066a3 on wbazant:master into 11b9554cab894f5ed6f9d88b53903ac17cba36ed on preaction:master.

preaction commented 4 years ago

This looks good at first glance. You're doing reactivity on the array values properly (following this VueJS docs). The only backend this will work on for the moment is Yancy::Backend::Static, but the rest should already have exceptions inside them for this case (unknown type). I can open a ticket to add JSON-based support for arrays to the relational databases (Pg, Mysql, Sqlite), and to figure out something for packing/unpacking DBIx::Class arrays (which... might just magically work).

I'm going to give this a real test, and maybe a Selenium unit test, before I merge it (probably tonight). Thanks!

preaction commented 4 years ago

This is now merged and I shall release it shortly. Thanks again!