WEEE-Open / tarallo

T.A.R.A.L.L.O. Inventory System
GNU Affero General Public License v3.0
16 stars 9 forks source link

Autosuggerst model brand #270

Closed Leone25 closed 1 year ago

Leone25 commented 1 year ago

Closes #147

This pull request switches forms inputs from content editable divs to standard html inputs, this brings multiple benefits including compatibility with the standard autocomplete library, one downside is that we loose the multiline capabilities, but if really needed we can consider using text areas for some features, everything else should work as ususal

lvps commented 1 year ago

@Leone25 That's great, contenteditable was so unreliable - but multi line has to work somehow, since we have multi line data in the database... Can you do something in JS, e.g. "if users press enter, the field turns into a textarea, and vice versa if they delete additional lines"? Or create a new feature type for features that are multiline (only notes at the moment, I think): Feature table > Type column, then you may need to run bin/generate-features. In some places a letter is used (S, I, D, E for String, Internet, Double and Enum) so add another one like M for Multiline. This requires a lot of changes (database, SQL CHECK thinghys, templates, editor.js, ...) but is probably the best way to do it

Leone25 commented 1 year ago

I was think the second option is better, I'll explore a little bit and see what I can do, perhaps we can make notes an always present input which uses text area and in case it's empty we just ignore it? idk, just trowing stuff out there

Leone25 commented 1 year ago

Ok I think I did it, I've done some testing and it seems to work properly, let me know what you think, I'm sure there is a bug somewhere since this change is quite "big" but I fixed everything I tested

lvps commented 1 year ago

@Leone25 The multiline part looks good to me, but I'll let @quadrollopo check that everything else is good and merge. Fortunately it seems it was easier than I thought. We also need to make some small change to the peracotta too, just so it doesn't crash with M features