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

feat: lnurl-auth onboarding #2754

Merged
merged 16 commits into from
Oct 5, 2023
Merged

feat: lnurl-auth onboarding #2754

merged 16 commits into from
Oct 5, 2023

Conversation

pavanjoshi914
Copy link
Contributor

@pavanjoshi914 pavanjoshi914 commented Sep 15, 2023

lnurl auth onboarding shifted in enable screen

Link this PR to an issue [optional]

Fixes #2713

Type of change

(Remove other not matching type)

  • feat: New feature (non-breaking change which adds functionality)

Screenshots of the changes [optional]

Add screenshots to make your changes easier to understand. You can also add a video here.

2023-09-15.10-41-24.mp4

Checklist

  • Self-review of changed code
  • Manual testing
  • Added automated tests where applicable
  • Update Docs & Guides
  • For UI-related changes
  • Darkmode
  • Responsive layout

@rolznz
Copy link
Contributor

rolznz commented Sep 18, 2023

image

Some issues on this dialog:

  • Copy is not so good in this case. We can now be more specific (we require a different onboarding component for LNURL auth?). Maybe we could say "A master key is required for Lightning Login"
  • Buttons should be bottom aligned

Base automatically changed from feat/onboarding-prompt to master September 18, 2023 13:20
@pavanjoshi914
Copy link
Contributor Author

// allowance is set, keys available for alby connector - no prompt direct login
// allowance is set keys not available for alby connector - open onboard
// allowance not set, keys available for ably connector - open login prompt
// allowance is not set, keys are not available for alby connector - open onboard
// allowance is  connector is different:  direct login
// allowance is not set, connector is different: open login prompt

@rolznz
Copy link
Contributor

rolznz commented Oct 4, 2023

@pavanjoshi914 when I disable login from the allowance, refresh the page and login I get an error:
image

@rolznz
Copy link
Contributor

rolznz commented Oct 4, 2023

@pavanjoshi914 when I disable login from the allowance, refresh the page and login I get an error: image

It also happens when I am using an LND connector, when I disconnect the site (completely remove the allowance), refresh, and then try to login again (on https://webln.twentyuno.net/login)

@pavanjoshi914
Copy link
Contributor Author

@pavanjoshi914 when I disable login from the allowance, refresh the page and login I get an error: image

It also happens when I am using an LND connector, when I disconnect the site (completely remove the allowance), refresh, and then try to login again (on webln.twentyuno.net/login)

Fixed in new commits

that happened because I was just calling the authPrompt, and not returning the response given by the authPrompt call. we need to return response back to content script else it will get a response as undefined.

@pavanjoshi914 pavanjoshi914 requested a review from rolznz October 4, 2023 06:48
Copy link
Contributor

@rolznz rolznz left a comment

Choose a reason for hiding this comment

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

tACK

@pavanjoshi914
Copy link
Contributor Author

instead of depending on connector type, now we depend on user wish to use mnemonic or not. the behaviour is::

for other connectors
a. if they have master key, and toggle enabled use master key for login
b. if they don't have key or toggle is disabled. use sign message which works by default for other connectors

  1. for alby connector
    if no master key . show onboarding
    if master key and toggle is off - fallback to sign message method, which will then not work which is expected

@rolznz rolznz merged commit 20d5b7d into master Oct 5, 2023
5 of 6 checks passed
@rolznz rolznz deleted the lnurl-auth-onboarding branch October 5, 2023 07:31
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.

[Feature] Alby should prompt user to setup a master key if using LNURL auth and none is set
3 participants