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 config table and analytics opt out on first load #39

Merged
merged 2 commits into from
Mar 10, 2022

Conversation

nathanleclaire
Copy link
Contributor

This was the last thing I wanted to get done before open sourcing. Users should be aware of analytics and able to opt out.

@@ -83,7 +83,6 @@ async function accounts(msg: AccountsRequest): Promise<AccountsResponse> {
await addKeypair(KEY_PATH);
}
const kp = await localKeypair(KEY_PATH);
logger.info('accounts', { net, pubKey: kp.publicKey });
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not relevant to PR but I don't think this adds much any more

@nathanleclaire
Copy link
Contributor Author

Screen Shot 2022-03-09 at 4 03 37 PM

such privacy

@SvenDowideit
Copy link
Member

I don't know what "Start" means - is that the "ok" button? I'm confused as to why there are 3 buttons on this page - can the toggle use a more standard UI for toggling?

@nathanleclaire
Copy link
Contributor Author

I don't know what "Start" means - is that the "ok" button?

Y, I can change to "OK"

I'm confused as to why there are 3 buttons on this page - can the toggle use a more standard UI for toggling?

reached for react bootstrap cause it was right there, but maybe over complicated, perhaps just a checkbox will do

@nathanleclaire
Copy link
Contributor Author

ah womp, 'switch' component was what i actually wanted to start and i didn't know that's what it was called

@nathanleclaire
Copy link
Contributor Author

Screen Shot 2022-03-10 at 11 27 37 AM

Copy link
Member

@SvenDowideit SvenDowideit left a comment

Choose a reason for hiding this comment

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

yass, LGTM

@SvenDowideit SvenDowideit merged commit b5116ce into main Mar 10, 2022
nathanleclaire added a commit that referenced this pull request Mar 10, 2022
@nathanleclaire nathanleclaire deleted the analytics_opt_out branch March 12, 2022 23:28
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