NicolasCARPi / jquery_jeditable

jQuery edit in place plugin. Extendable via plugin architecture. Plugins for plugin. Really.
https://jeditable.elabftw.net
MIT License
1.74k stars 458 forks source link

Passing sorted select list as list of tuples #208

Closed michal-juranyi closed 5 years ago

michal-juranyi commented 5 years ago

Hi there, loading list item to select from URL is great feature, but sorting mechanisms are a bit hacky I think :)

Documentation is talking about JSON encoded arrays, which is not true - in fact those are JSON encoded objects (might be called associated arrays, but I think it's misleading in this situation). JSON objects are unsorted by design.

RFC7159 says:

An object is an unordered collection of zero or more name/value pairs, where a name is a string and a value is a string, number, boolean, null, object, or array.

This plugin, however, relies on order of elements in such object, which brings problems, since many JSON libraries encode objects in arbitrary order and moreover, those libraries are encoding data structres in particular languages which are also unsorted by design (i.e. dictionaries in Python).

Ordering by either key or values is not always an option (i.e. ordering whole list by value but one item which I'd like to be the first one) and writing another sorting function on frontend might be double work in case I already have sorted items on backend (i.e. loaded ordered from DB).

Therefore, to bring some standards into this problem I propose this solution:

Extend/modify content function of select type to accept JSON arrays as defined in RFC, which will consist of tuples (2-item array) representing keys and values and will respect its order.

Example:

[
  ['val1', 'label1'],
  ['val2', 'label2'],
  ['val3', 'label3']
]

above JSON would result in same ordering as following one does now, but would comply with RFC:

{
  'val1': 'label1',
  'val2': 'label2',
  'val3': 'label3'
}

I can prepare PR for this.

NicolasCARPi commented 5 years ago

Hello,

Thank you very much for your contribution, it is appreciated. :+1:

It would be even better if you could include tests for this. With the different kind of input one can have for "select", and checking that the element is properly assembled from it.

Regards, ~Nico

michal-juranyi commented 5 years ago

Glad to hear that :) I've just pushed some tests. There are basically 3 types of input that I've identified: key-value object, array of strings (int ids are autoassigned) and new array of arrays.

I don't think that those tests are exhausting, but might be sufficient for this case.

NicolasCARPi commented 5 years ago

Thanks! Merged!