FLEXIcontent / flexicontent-cck

Advanced content management for Joomla
http://www.flexicontent.org
82 stars 53 forks source link

Fix for https://github.com/FLEXIcontent/flexicontent-cck/issues/969 multiselect sortable field bug #974

Closed rubenreyes2000 closed 4 years ago

rubenreyes2000 commented 4 years ago

This code properly handles the values order for a multiselect sortable field with saved values. Bug is caused because the default values is always loaded, and ignores the order already saved by user.

micker commented 4 years ago

@ggppdk ?

ggppdk commented 4 years ago

This used to work, i will check why this is broken now

rubenreyes2000 commented 4 years ago

You were not able to reproduce the error? Did you see the description in https://github.com/FLEXIcontent/flexicontent-cck/issues/969?

rubenreyes2000 commented 4 years ago

You can see the bug in action in this video, on a fresh installation: https://www.loom.com/share/aa9353679d444e07bc3cc1e2ab6b7068 you can skip to 1:39 to see the field settings and the watch the error happening after the last item save

ggppdk commented 4 years ago

Sorry, I closed this PR by mistake

ggppdk commented 4 years ago

The PR was closed, because i deleted the branch, but i can still see your suggested fix, will review this soon

ggppdk commented 4 years ago

Re-ordering the options before load has some advantages , with only disadvantage changing default order, which may not be bad

Just , i have reviewed this and i could not replicate it i have configured to have default values but i still see the values i have re-ordered after form reloads

Still because we want to replace select2 (as we can not use select2 JS v4 because it is too SLOW !!!!) This maybe useful

I wonder why this is failing. Do you have an online testing website that shows this is issue, can you send me login to such a -testing- website via email ?

micker commented 4 years ago

Yep maybe its time to check choice.js because it Will be added to joomla 4 ?

ggppdk commented 4 years ago

Yes, i have done some evaluation of it and i have mentioned it before somewhere that it seems to be our best option for replacement is choice.js

rubenreyes2000 commented 4 years ago

@ggppdk did you follow all the steps listed in https://github.com/FLEXIcontent/flexicontent-cck/issues/969 to reproduce the error?

ggppdk commented 4 years ago

Thanks, i have tested your suggested fix and committed it