User tests: Successful: Unsuccessful:
Pull Request for Issue #33861.
This PR fixes the issue #33861
Status | New | ⇒ | Pending |
Category | ⇒ | JavaScript Repository NPM Change |
Labels |
Added:
NPM Resource Changed
?
|
you have found bug correctly, but the fix not really ;)
You have to skip "mirror" row, not last row.
The mirrored row is removed in "dragend" event, it would be best to call the request here, however this event also triggered when user cancel sorting, so it a bit tricky.
Just make the loop like this:
for (i = 0, l = rows.length; l > i; i += 1) {
// Skip a mirror element
if (rows[i].closest('.gu-mirror')) continue;
rows[i].value = i + 1;
result.push(`order[]=${encodeURIComponent(rows[i].value)}`);
result.push(`cid[]=${encodeURIComponent(inputRows[i].value)}`);
}
@Fedik No surprise with my beginner JS level :D. I asked @dgrammatiko before and he confirmed the fix looks OK. I tested and it seems it works, too. Maybe you can help with a proper fix? I will be happy to close this PR
just copy paste the code from my last comment ;)
Hi Tuan, Although this fixes the issue initially reported here: #33861 testing this a bit further exposes another bug in the logic:
create 6 custom fields, set the view list length to 5 (so only the first 5 are shown).
now when sorting on the first page, the sorting looks okay until you F5 then field number 6 turns out to be not the last but the first...
Should I create a new issue for this?
to fix eslint, add
// Skip a mirror element
// eslint-disable-next-line no-continue
if (rows[i].closest('.gu-mirror')) continue;
this no-continue
rule is pointless, I would disable it globally
@Ruud68 I'm unsure If my js skill is enough to address that. So just leave this PR here for a while, If I cannot get it sorted, I will close and ask @Fedik or @dgrammatiko for help
One thing I don't understand is why ordering of fields are set to 0 when it is created? If we create 3 fields, I would expect ordering of the fields set to 1, 2, 3..... and so on. For case, I think the last field has ordering set to 0, so after the sorting, it still has ordering 0, while other fields on the first page has ordering set to right value and it causes the error
So maybe something related to field ordering when it is first saved?
So maybe something related to field ordering when it is first saved?
This something in PHP side on save, I suspect.
This something in PHP side on save, I suspect.
I think so. But we might also have problem with client side, too. If from second page, we start ordering again with value 0 (or 1) for first row, we would get the issue (need to look at more carefully, just a quick analyst)
makes me wonder what happens when you have a list of e.g. 100K items you sort, would it rewrite all the records ordering fields?
Just a thought as I have a customer who has that amount of sortable items...
As I guessed, on new page (second, third page....), we re-start with ordering of items again from 1, so we have a big bug here :( .
And to further thicken the plot: in J3 ordering is using negative numbers, so e.g. when you set field one below field 2 then field one gets ordering of 1 and field 2 gets ordering of -1.
It should also be tested how ordering of fields will handle these when upgrading to 4.0 and then reordering n 4. Will it correctly handle the negative numbers etc. as the logic of ordering has apparently changed.
we re-start with ordering of items again from 1
hmhm the script just send the final positions to the backend, and backend should recalculate the final order for given ID, depend from element position in request.
In theory.
Not sure how it work now, in backend.
Not sure how it work now, in backend
From my quick look, the posted ordering will be saved for each record (if the value for that change), then the ordering if related items will be re-calculated (I haven't looked at that re-calculate logic yet)
So from page 2 and page 3 for example, the item will have ordering again, set to 1, 2, 3......, so it is completely wrong. We need to look at it and correct it in a separate PR
I have tested this item
@sandramay0905 Could you please re-test this PR ? Just makes sure it works like in your previous test. There is another bug related to re-ordering items but we will address it in a different PR
I have tested this item
Status | Pending | ⇒ | Ready to Commit |
Thanks for testing !
Thanks for testing !
Status | Ready to Commit | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2021-05-17 16:40:35 |
Closed_By | ⇒ | Quy | |
Labels |
Added:
?
|
Thank you!
I have tested this item✅ successfully on 34b45a0
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/33909.