Use sessionStorage for viewingPatternData and activePattern

[kasparsj]

localStorage, which is shared between tabs, was being used for these, and although it wasn't [live] synchronised between tabs, it would e.g. restore to the last saved data (accross all tabs) upon refresh/reload/new window.

sessionStorage is separate for each tab, but persists tab data across refreshes.

this should fix #1049 as it's tracking the data about which pattern is active/selected for each tab separately.

[felixroos]

ah yes thanks for spotting that! one idea that might simplify the implementation: maybe it's possible to use a regular nanostores atom and update a session storage field on change (using .listen). The default value can be the session storage field.

[felixroos]

i meant this:

const initialActivePattern = sessionStorage.getItem('activePattern');
const $activePattern = atom('activePattern', initialActivePattern);
$activePattern.listen((value) => sessionStorage.setItem('activePattern', value));

does that make sense?

[kasparsj]

I see, my idea was to keep the api same as persistentAtom and base the implementation on this : https://github.com/nanostores/persistent/pull/30 (no idea why it wasn't merged)

[felixroos]

I see, my idea was to keep the api same as persistentAtom and base the implementation on this : https://github.com/nanostores/persistent/pull/30 (no idea why it wasn't merged)

atom is also a Store type (WritableAtom) which works for the part of the API that we use (set, get and passing it to useStore) so I don't see why we need more complexity here

[kasparsj]

please check now...

• kept it in a function, because there are 2 calls
• kept the sessionStorage !== undefined check, to not error in env where it's not supported e.g. testing env etc
• also kept the newValue === undefined check, since it's what the atom docs say, how to delete

[felixroos]

thanks, this now looks good. In the future viewingPatternData might be refactored a little bit to not need parsing on every render (PatternsTab > parseJSON), but I guess that can be done in a separate PR. Also I wonder if a browser that doesn't support sessionStorage can even run the rest of strudel :P

[yaxu]

Is this ready to merge?

[felixroos]

Is this ready to merge?

yup! thanks @kasparsj