Skip to content

Manage localstorage cookies#720

Open
aswallace wants to merge 7 commits intomainfrom
feature/localstorage-cookies
Open

Manage localstorage cookies#720
aswallace wants to merge 7 commits intomainfrom
feature/localstorage-cookies

Conversation

@aswallace
Copy link
Copy Markdown
Contributor

@aswallace aswallace commented Apr 6, 2026

Context

Although we did have a variable in state for storing whether a user has visited the app before, we weren't actually using it. This meant that the tutorials were showing up every time a user navigated from the website to the app, instead of just their first time.

Changes

Implements PersistentConfigService for web (previously only implemented for desktop) so that we can manage localstorage cookies.
Also adds a hook in the app that both updates the "has used app before" cookie and launches the tutorials. This means that the tutorials now also launch when a first-time user is sent a link that goes directly to the app itself.

Testing

Manually verified (in incognito) that the tutorials launch when it's a user's first time visiting the app, and not any following times.

@aswallace aswallace changed the title Feature/localstorage cookies Manage localstorage cookies Apr 6, 2026
@aswallace aswallace marked this pull request as draft April 6, 2026 21:12
// Specifically added for WebWorker API, but seems that listing that one
// makes adding ESNext also necessary
"lib": ["WebWorker", "ESNext"],
"lib": ["WebWorker", "ESNext", "DOM"],
Copy link
Copy Markdown
Contributor Author

@aswallace aswallace Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just because of some typescript errors showing up after a recent IDE update; makes this library explicitly available

@aswallace aswallace marked this pull request as ready for review April 6, 2026 21:35
@aswallace aswallace requested review from kmitcham and pgarrison April 6, 2026 21:36
@aswallace aswallace self-assigned this Apr 6, 2026
Copy link
Copy Markdown
Contributor

@SeanDuHare SeanDuHare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - curious what you think about the versioning though so just waiting for that

}

// Use browser localstorage to persist data between sessions
export default class PersistentConfigServiceWeb implements PersistentConfigService {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The other config storage keeps track of versions with schemas right? What do you think about storing the config version whenever this initializes so that we can version the config version (..) style going forward in case we end up changing the config variables a lot in the future? We could then, if the major version has changed, delete(?) the cookies saved so far and start anew. Worth checking if there is some existing convention for how apps do this too if you haven't yet.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense to me! The other version was specific to "electron-store", and I wasn't sure if there was an equivalent I could translate to local storage. I think it's worth doing though, will take a look.

On a similar topic, I didn't include the other cookies that we currently use in desktop, but may be worth discussing if we want any of those to be included in web (e.g., columns?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants