carbon-design-system / carbon-components-svelte

Svelte implementation of the Carbon Design System
https://svelte.carbondesignsystem.com
Apache License 2.0
2.69k stars 261 forks source link

LocalStorage key not reactive #1204

Open wilriker opened 2 years ago

wilriker commented 2 years ago

LocalStorage's key is not reactive which leads to updating the wrong value in localStorage when the key is updated without a full destroy and mount cycle.

metonym commented 2 years ago

Can you provide a minimal repro that illustrates your use case?

wilriker commented 2 years ago

So here is my minimal example: https://svelte.dev/repl/9fec00f7fa0c44df93da413d6c07893b?version=3.46.4

Note that in TableWithPersistedPaging the LocalStorage is commented because in the REPL it will throw an error because of sandboxing otherwise.

Also the REPL out of some reason does not render the Pagination correctly.

Either way, here is an explanation of what this is supposed to do: I have a custom Table wrapping a DataTable that always has Pagination added and I do persist the currently selected page and pageSize into local storage. And this needs to be done for each and every table even if there can be multiple tables shown at the same page.

In the main table I use the expandable property to let the user expand several columns - each column will render its own table.

If a user expands column A they will see a table with data for that row in column A. When the user now clicks on column B in the same row (that part is important) without collapsing the already visible expanded row (equally important) than Svelte will update the ID of the table as well as all relevant data displayed in the table but the key for LocalStorage won't be affected and thus it will continue to update the persisted pagination settings for column A instead of column B.

If the user either expands any column in another row or first collapses the currently expanded row then the inner table will undergo a destroy-render cycle and the LocalStorage component sees the new key and everything works.

It also works by simulation the user collapsing the column first by extending the expand function to first set expandedRowIds to an empty array and then setting the correct values in a setTimeout(() => { ... }, 1 but that has an expected but rather unpleasant visual effect of collapsing and expanding the same row again.