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

Add ability to init the wallet with a predefined allowed origin #448

Merged
merged 10 commits into from
Dec 4, 2023

Conversation

Tbaut
Copy link
Collaborator

@Tbaut Tbaut commented Nov 30, 2023

closes #433

This allows to reduce a lot of the clicks to connect etc if not needed. Users once they've allowed a website to access their accounts will not have to authorize the connection every single time. This is what this feature enables.

The visitWithLocalStorage function which was used all over, now must have a new name. Please suggest anything better if you think visitCustom isn't great. The reason why I had to bloat it, is because the extension and the accounts need to be loaded in the window object before the page loads, which is also when we set the custom localStorage elements 🤷‍♂️

@asnaith
Copy link
Contributor

asnaith commented Nov 30, 2023

The visitWithLocalStorage function which was used all over, now must have a new name. Please suggest anything better if you think visitCustom isn't great.

As the setup is done before the visit perhaps we could name it this way around?

  • setupAndVisit
  • configureAndVisit
  • injectConfigAndVisit (I like this one but it's long 😅)

@Tbaut
Copy link
Collaborator Author

Tbaut commented Nov 30, 2023

That failing test is annoying. I can reproduce locally, and it sounds like it's actually a bug, but it's not happening in prod,

@Lykhoyda
Copy link
Collaborator

Lykhoyda commented Dec 1, 2023

The visitWithLocalStorage function which was used all over, now must have a new name. Please suggest anything better if you think visitCustom isn't great.

As the setup is done before the visit perhaps we could name it this way around?

  • setupAndVisit
  • configureAndVisit
  • injectConfigAndVisit (I like this one but it's long 😅)

Agree, we need to come up with a better name for visitCustom method. Maybe visitWithConfig or something else

@asnaith
Copy link
Contributor

asnaith commented Dec 1, 2023

setupAndVisit it is :)

Looks good to go

@Tbaut
Copy link
Collaborator Author

Tbaut commented Dec 1, 2023

ah I wrote a comment the other day, at least I thought, but didn't click and commit. I went for setupAndVisit because "inject" felt weird to me to talk about localStorage. Also, keeping it generic lets us add many other things in the future, hopefully without renaming.

now weirdly the test that used to fail is passing... 🙄 if it proves annoying in the future I'll just take another look at it, but it sounded a little like an annoying edge case specific to the test.

@Tbaut
Copy link
Collaborator Author

Tbaut commented Dec 1, 2023

I just had to name it for it to fail. I'll take a look then, before merging.

@Tbaut Tbaut merged commit fc41d47 into main Dec 4, 2023
6 checks passed
@Tbaut Tbaut deleted the tbaut-allow-init-with-origin branch December 4, 2023 14:50
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.

Refactor the Cypress wallet plugin
3 participants