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

v1.0.0-beta.26 #103

Closed
wants to merge 8 commits into from
Closed

v1.0.0-beta.26 #103

wants to merge 8 commits into from

Conversation

pedrouid
Copy link
Contributor

@pedrouid pedrouid commented Jan 16, 2020

  • deprecate individual connectors
  • updated README for readability
  • expose instantiated libraries
  • cache preferred provider in localStorage
  • update dependencies in example

@pedrouid pedrouid mentioned this pull request Jan 16, 2020
@pedrouid
Copy link
Contributor Author

Fix #68
Fix #102

Copy link
Contributor

@crisgarner crisgarner left a comment

Choose a reason for hiding this comment

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

The code seems fine, I wasn't able to test it due to an
Allocation failed - JavaScript heap out of memory error. This is an issue with my computer, I'll try to fix it in order to do a full test

@pedrouid
Copy link
Contributor Author

the build error is probably related to some dependency upgrade on the example app

@pedrouid
Copy link
Contributor Author

We need to discuss the cache preferred provider cc @crisgarner

Currently all versions until 1.0.0-beta.25 did NOT cache the user's preferred provider by default

So we should probably disable it by default to make it backwards compatible and document it for developers to enable themselves

@crisgarner
Copy link
Contributor

We need to discuss the cache preferred provider cc @crisgarner

Currently all versions until 1.0.0-beta.25 did NOT cache the user's preferred provider by default

So we should probably disable it by default to make it backwards compatible and document it for developers to enable themselves

Sure, I don't think it will be a problem since a lot of devs have been asking for this, but it makes sense for people that just update libraries and don't read the docs. Do we need a way to delete the session? It might be helpful to create some sort of sign-in / sign out. 🤔

@pedrouid
Copy link
Contributor Author

I added clearPreferredProvider method to remove the cached provider so that it will allow developers to reset it

Although since this library's main purpose is to handle UI/UX we should actually build this as part of the modal as a prompt for the user: "Remember chosen provider". The toggle would default to false and the user could to remember it for 30 days

Also instead of triggering the preferred provider on toggleModal, maybe it would be better to listen to the page load and trigger a connect event right after.

Thoughts?

@pedrouid
Copy link
Contributor Author

Maybe we should cut a release with everything except the preferred provider stuff for now

@pedrouid
Copy link
Contributor Author

Replaced by #106

@pedrouid pedrouid closed this Jan 20, 2020
@pedrouid pedrouid deleted the update-readme branch January 20, 2020 17:07
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.

2 participants