ciena-frost / ember-frost-table

MIT License
1 stars 9 forks source link

When `itemKey` attributes is omitted, can't deselect the selected row. #52

Open quincyle opened 6 years ago

quincyle commented 6 years ago

unable_to_deselect_row

juwara0 commented 6 years ago

@quincyle - Thank you for opening an issue to track this. Can you provide the following details about this issue:

quincyle commented 6 years ago
notmessenger commented 6 years ago

@quincyle itemKey is a required property for the Selection API (https://ciena-frost.github.io/ember-frost-table/#/selection) to function correctly (many occurrences of itemKey in this code - https://github.com/ciena-frost/ember-frost-table/blob/f5961a26fc31248a76541067781bb7ae8394b7e9/addon/mixins/selection.js#L59)

How do you propose maintaining the same functionality with this value not being set? Do you have a specific scenario in which the population of the itemKey property is problematic for you?

As it currently stands this issue is not one that will be addressed but I am open to it being shown to me what I am not understanding about the situation.

quincyle commented 6 years ago

I don't have a problem to populate an itemKey and feed it into table, but the circumstance of it is being required doesn't seem correct to me.

itemKey was initially introduced to solve the problem of shallow comparison. Imaging we have record1 being selected, later on, the UI receives a new copy of record1 via polling, but these two objs are not the same object from javascript perspective. Now we have a hard time to compare them and retain the selection state of UI.

But thanks for ember-data, we don't need to deal with this headache very often as ember-data wraps the response coming from the server in its record model and reuse the same object before and after the polling.

The itemKey concept was first implemented in ember-frost-list. By default, list will perform a shallow comparison, but we also expose the configurable itemKey, so only list records with a matching itemKey will update when itemKey is set.

This behavior changes when it gets pulled into ember-frost-table, as itemKey becomes a required field for selection to work. But as @notmessenger you mentioned, this is actually indicated in the doc of ember-frost-table. (I didn't notice it) So I will consider it as an inconsistent behavior instead of a bug.

I can raise a PR to make it align with ember-frost-list, or we can leave it as so.