Skip to content

Store API data in local storage instead of sync storage. #122

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.

Sign up for GitHub

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

Jump to bottom
Open
FozzieHi wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Store API data in local storage instead of sync storage. #122

FozzieHi wants to merge 3 commits into from

Conversation

@FozzieHi
Copy link

@FozzieHi FozzieHi commented Feb 13, 2022 •
edited
Loading

Closes #121

  • Stores apiUrl and apiKey in local storage instead of sync storage. We use a separate TYPES variable for this instead of making SETTINGS an object for ease of use when using SETTINGS keys.
  • On startup, moves any existing apiUrl or apiKey values in sync storage to local storage and deletes them from sync storage if successful.

@ngxson ngxson requested review from nguyenkims and ngxson February 13, 2022 10:15
ngxson
ngxson requested changes Feb 13, 2022
View reviewed changes
Copy link
Collaborator

@ngxson ngxson left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Hi FozzieHi. Thank you for your contribution! Your suggestions and this PR look good to me 👍 👍 . I only have a small comment that need to be resolved. Thank you.

@FozzieHi FozzieHi requested a review from ngxson February 13, 2022 10:59
@FozzieHi FozzieHi mentioned this pull request Feb 13, 2022
Closed
ngxson
ngxson approved these changes Feb 14, 2022
View reviewed changes
Copy link
Collaborator

@ngxson ngxson left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you!

@ngxson
Copy link
Collaborator

ngxson commented Apr 22, 2022

@nguyenkims Can you please take a look to see if we can merge? Thank you!

@nguyenkims
Copy link
Contributor

nguyenkims commented Apr 23, 2022

@FozzieHi @ngxson the PR look good to me 👌. Did you have a chance to test it on Firefox and Chrome?

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

Reviewers

@ngxson ngxson ngxson approved these changes

@nguyenkims nguyenkims Awaiting requested review from nguyenkims nguyenkims is a code owner

@acasajus acasajus Awaiting requested review from acasajus acasajus is a code owner

@cquintana92 cquintana92 Awaiting requested review from cquintana92 cquintana92 is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

After logging into Firefox account on brand new device, SimpleLogin extension automatically logs in too

3 participants

@FozzieHi @ngxson @nguyenkims