Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request persistent storage #701

Merged
merged 2 commits into from
Dec 20, 2023
Merged

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Dec 20, 2023

Related to #411

TODO:

  • only prompt for persistent storage permission if notifications enabled

@ekzyis ekzyis added the bug label Dec 20, 2023
@huumn
Copy link
Member

huumn commented Dec 20, 2023

Does this only do this if/when notifications are enabled? I don't want it prompting people potentially for no reason. Best to scope it to where we need it.

@ekzyis
Copy link
Member Author

ekzyis commented Dec 20, 2023

Does this only do this if/when notifications are enabled? I don't want it prompting people potentially for no reason. Best to scope it to where we need it.

It didn't prompt me but I guess I missed the Firefox part:

Firefox requests permission from the user to use persistent storage. Chromium-based browsers give or deny persistence based on a heuristic to determine the importance of the content for the user. One criteria for Google Chrome is, for example, PWA installation.

So good catch, the prompt (and other browsers) wasn't on my mind:

firefox:
2023-12-20-200919_690x202_scrot

@ekzyis ekzyis marked this pull request as draft December 20, 2023 18:52
@ekzyis ekzyis force-pushed the 411-persistent-storage branch from 38ea50e to 7295420 Compare December 20, 2023 19:18
@ekzyis ekzyis marked this pull request as ready for review December 20, 2023 19:19
@huumn huumn merged commit 056be01 into stackernews:master Dec 20, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants