Skip to content

Commit 966a79a

Browse files
petschkiclaude
authored andcommitted
fix(pat-structure): keep table rows in collection order
The table row rendering iterated the collection with an async callback (`collection.each(async ...)`) that appended each row only after awaiting `view.render()`. Row rendering is async (it awaits icon resolution), so rows were appended in the arbitrary order their render promises resolved rather than in collection order, corrupting the sort order of the folder contents. This stayed hidden until 5.6.0: DataTables was initialized with `order: [0, "asc"]`, which re-sorted the rows by the hidden `_sort` column (the server order index) on every draw and thus masked the race. Commit 57cd7dd ("Fix ordering logic.") changed this to `order: []`, removing the safety net and exposing the underlying bug. Fix the root cause: build the row views in collection order, render them all in parallel via `Promise.all`, then append them in order. This guarantees the DOM order matches the collection order regardless of the DataTables ordering settings. The now-obsolete `table_row_rendering_finished` event dance and its `events` import are removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3434c84 commit 966a79a

1 file changed

Lines changed: 11 additions & 33 deletions

File tree

src/pat/structure/js/views/table.js

Lines changed: 11 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import $ from "jquery";
22
import _ from "underscore";
33
import _t from "../../../../core/i18n-wrapper";
4-
import events from "@patternslib/patternslib/src/core/events";
54
import registry from "@patternslib/patternslib/src/core/registry";
65
import BaseView from "../../../../core/ui/views/base";
76
import TableRowView from "./tablerow";
@@ -125,10 +124,8 @@ export default BaseView.extend({
125124
if (this.collection.length) {
126125
const container = $("tbody", this.$el);
127126

128-
const collection_length = this.collection.length;
129-
let collection_cnt = 0;
130-
131-
this.collection.each(async (result) => {
127+
// Create one row view per result, keeping the collection order.
128+
const rowViews = this.collection.map((result) => {
132129
this.dateColumns.map((col) => {
133130
// empty column instead of displaying "None".
134131
if (
@@ -139,41 +136,22 @@ export default BaseView.extend({
139136
}
140137
});
141138

142-
const view = new TableRowView({
139+
return new TableRowView({
143140
model: result,
144141
app: this.app,
145142
table: this,
146143
});
147-
await view.render();
148-
container.append(view.el);
149-
150-
// Throw the ``table_row_rendering_finished`` event after all table rows have finished rendering.
151-
collection_cnt++;
152-
if (collection_cnt === collection_length) {
153-
this.el.dispatchEvent(new Event("table_row_rendering_finished"));
154-
}
155144
});
156145

157-
// NOTE: this is based on the concept of awaiting an event.
158-
// See this Stackoverflow answer here:
159-
160-
// https://stackoverflow.com/a/44746691/1337474
161-
// When the last table row has finished rendering, throw an event.
162-
// For this "table_row_rendering_finished" event we're a-waiting for.
163-
// And after that we can scan the table.
164-
const table_row_rendering_finished = () =>
165-
new Promise((resolve) =>
166-
events.add_event_listener(
167-
this.el,
168-
"table_row_rendering_finished",
169-
"table_row_rendering_finished__listener",
170-
resolve,
171-
{ once: true }
172-
)
173-
);
146+
// Render all rows in parallel (row rendering is async because of
147+
// icon resolution), but wait for all of them to finish before
148+
// appending. Appending in ``rowViews`` order guarantees the DOM
149+
// order matches the collection order – otherwise rows would be
150+
// appended in the arbitrary order their async render resolves,
151+
// which corrupts the sorting.
152+
await Promise.all(rowViews.map((view) => view.render()));
153+
rowViews.forEach((view) => container.append(view.el));
174154

175-
await table_row_rendering_finished();
176-
events.remove_event_listener = (this.el, "table_row_rendering_finished__listener"); // prettier-ignore
177155
registry.scan(this.$el);
178156

179157
// Set the context (again) after rerendering. If render was called after a collection sync – which is the

0 commit comments

Comments
 (0)