Addepar / Addepar/ember-table

`selectionMatchFunction` isn't respected in TableRowMeta `select` function

Open
#1,160 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
JavaScript
Stars
1.7k
Forks
351
Avg merge
1d 22h
Merged PRs (30d)
1

Description

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:
https://github.com/Addepar/ember-table/blob/f9d05b3e5417519ea19ec7398c69276053c1f06d/addon/-private/collapse-tree.js#L188-L197

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.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in addon/-private/collapse-tree.js around lines 188-197 and trace TableRowMeta.select alongside the selectionMatchFunction logic used by isSelected and isGroupSelected. Reproduce selection with recreated POJO rows, paging or refresh, and range selection. Done means selecting and unselecting uses the matching function consistently without duplicate or lingering selected rows.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
frontend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.