flekschas/piling.js

Add support for adding and removing items and id-based matching

Closed this issue · 3 comments

It should be possible to dynamically add and remove items from the library based on their id.

E.g.:

const items = new Array(10).fill().map(() => ({ src: createRandomLinePlot() }));
const piling = createPilingJs(element, { items });

// This should add another 10 items
const newItems = new Array(10).fill().map(() => ({ src: createRandomLinePlot() }));
piling.set('items', [...items, ...newItems] });

// This should remove the newly added 10 items
piling.set('items', items });

Currently, the id is bound to the index. Therefore, when we call piling.set('items', items }); we essentially tell the library to remove all piles with an index greater than 9. If we had previously called piling.set('items', [...newItems, ...items, ] }); instead and then piling.set('items', items });, then we would have additionally re-rendered items two more times: the first time because the library thinks that items get updated to newItems and the second time because the library thinks newItems get updated back to items.

This behavior can be too inflexible so it should be possible to specify a custom id directly, e.g.

const items = new Array(10).fill().map((x, i) => ({
  id: `originalItem${i}`,
  src: createRandomLinePlot()
}));
const piling = createPilingJs(element, { items });

// This should add another 10 items
const items = new Array(10).fill().map((x, i) => ({
  id: `newItem${i}`,
  src: createRandomLinePlot(
}));

// Now the order of items shouldn't matter anymore as we defined custom IDs!
piling.set('items', [...newItems, ...items] });

// Due to the custom, we know that we only have to remove `newItems` and keep`items`
piling.set('items', items });

Hi Fritz, I'm confused here. Would we use the custom id as the id in renderedItems and pileInstances, or it's just a property in the items state?

If it's just a property, then the item index in the items state maybe not equal to the index(key) in the renderedItems, since we initially use the index of items state as the key.
Like in your example,

piling.set('items', [...newItems, ...items] });
piling.set('items', items });

The items' index would be different in the items state and in renderedItems.

If it's the former case, what if some items have custom-id and some don't?

I just got really confused when thinking about how to update state.items 😵

The items' index would be different in the items state and in renderedItems.

I am not sure why this would be the case. When we store rendered items we should use their id. E.g., renderedItems.set(itemId, itemInstance). By default itemId should be the index of the item but it should also be possible for the user to provide a custom id.

When updating state.items we need to match items either by their id property or their index. The easiest way to efficiently support both matching types is to switch from an array of items to an object of items. This way we can do

newItems.forEach((newItem, index) => {
  if (oldItems[newItem.id || index]) {
    // item updated
  } else {
    // add new item and potentially set `index` as the item's `id` if it doesn't have one!
  }
});

(Please take a look at setItems() in pile.js how I dealt with outdated items. You can use the same strategy.)

Now you might wonder "but in the index case, there is no id" property. Well, the simplest solution is to just add the index as the id property to be explicit.

This was addressed in #111 and #121