Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

selectionMatchFunction isn't respected in TableRowMeta select function #1160

Open
daniellubovich opened this issue Jan 12, 2025 · 0 comments

Comments

@daniellubovich
Copy link

Use Case

I'm building a table that is populated by rows that are POJOs which combine info from multiple E-D models. Example psuedocode:

loadData = task(async () => {
  const data1 = await this.store.query('model1', {});
  const data2 = await this.store.query('model2', {});

  return data1.map(d => {
    return { id: data1.id, name: data2.name };
  });
});

get rows() { return this.loadData.lastSuccessful.value ?? []; }

This table allows the user to refresh the current page, change pages, and select multiple items.

The Issue I'm Seeing

Since we recreate the rows whenever the paging or refresh occurs, the references will no longer match what may have been in the previous selection array.

So, I found selectionMatchFunction which was added ~v3.0 which is seemingly meant to address this issue. It works for the most part - it correctly renders rows as selected based on my own criteria defined in selectionMatchFunction (compare ID instead of object ref).

The issue is that the select function in the TableRowMeta class doesn't seem to respect the selectionMatchFunction. I think there are two main areas here to address:

  1. The use of Set to maintain uniqueness of selected elements is always going to do an object ref compare
  2. Even if we converted the Set to an Array, we'd need to update any functions that add/remove elements or do equality checks to use selectionMatchFunction in the same way we do for the isSelected and isGroupSelected CPs.

For example, in the range selection branch within this function we add the rows to the Set, assuming that any dupes will be removed, without asking selectionMatchFunction if they are actually dupes:

let { _lastSelectedIndex } = tree;
let isFirstIndexDefined = typeof _lastSelectedIndex === 'number';
let minIndex = isFirstIndexDefined ? Math.min(_lastSelectedIndex, rowIndex) : rowIndex;
let maxIndex = isFirstIndexDefined ? Math.max(_lastSelectedIndex, rowIndex) : rowIndex;
for (let i = minIndex; i <= maxIndex; i++) {
selection.add(tree.objectAt(i));
}

The result of this is that we end up with either 1) dupes in the selection array, or 2) elements not being un-selected when they should be.

Brain dump over - I hope I explained this well enough. When I get a few minutes this week I'll put together a gist. If this seems like a real issue to folks, I can put together a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant