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

frontend: create dialog on remember wallet #3085

Conversation

shonsirsha
Copy link
Collaborator

Show dialog when enabling remember wallet to better inform users about what the feature is. The dialog is an element itself called EnableRememberWalletDialog.

This dialog is (also) controlled by a new config variable called hideEnableRememberWalletDialog.

Also created DisableRememberWalletDialog component (just a refactor).

Comment on lines +18 to +22
useEffect(() => {
if (config && config.frontend) {
setShouldNotShowDialog(config.frontend.hideEnableRememberWalletDialog);
}
}, [config]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We use this pattern (useLoad+getConfig and useEffect+localstate) quite a lot.

Prob makes sense to create a reusable hook for this e.g useConfig(config.frontend.someconfig). It can just return that 'local state', to be used in the component that uses this useConfig hook.

Copy link
Collaborator

@thisconnect thisconnect Dec 3, 2024

Choose a reason for hiding this comment

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

since we already get the config in app provider what do you think about having the config in the context ?

One thing to consider is we might need to mock the config for tests, but I guess we can mock the config anyway no matter if it is in context or coming from a hook, wdyt?

Copy link
Collaborator

@thisconnect thisconnect left a comment

Choose a reason for hiding this comment

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

tested LGTM 👍

Show dialog when enabling remember wallet to better inform users
about what the feature is. The dialog is an element itself called
EnableRememberWalletDialog.

This dialog is (also) controlled by a new config variable called
hideEnableRememberWalletDialog.

Also created DisableRememberWalletDialog component (just a
refactor).
@shonsirsha shonsirsha force-pushed the frontend-remember-wallet-enable-dialog branch from 34ecdb9 to 7789b86 Compare December 3, 2024 12:56
@shonsirsha shonsirsha merged commit 44f3c76 into BitBoxSwiss:master Dec 3, 2024
6 of 7 checks passed
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