Skip to content
This repository was archived by the owner on Feb 23, 2021. It is now read-only.

Conversation

@valentinewallace
Copy link
Contributor

@valentinewallace valentinewallace commented Jul 11, 2019

Closes #1167. Tested in an iOS build. I didn't add the feature to desktop.

@valentinewallace valentinewallace changed the title Dev/atpl choice setup Select autopilot during wallet setup. Jul 11, 2019
@valentinewallace valentinewallace requested a review from tanx July 12, 2019 00:50
</Text>
<View style={styles.list}>
<SettingItem
name="Use autopilot (recommended)"
Copy link
Contributor Author

@valentinewallace valentinewallace Jul 12, 2019

Choose a reason for hiding this comment

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

I added the word "recommended" because I think it's an improvement. Feel free to request removal if you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense 👍

@tanx tanx force-pushed the dev/atpl-choice-setup branch from 1d2c25c to c82ca59 Compare August 19, 2019 14:00
Copy link
Contributor

@tanx tanx left a comment

Choose a reason for hiding this comment

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

@valentinewallace I rebased this on master and made some suggestions...

<View style={iStyles.left}>
<Text style={iStyles.name}>{name}</Text>
{copy ? <Text style={iStyles.copy}>{copy}</Text> : null}
</View>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would probably be better to create a separate SettingCopyItem component than trying to squeeze both use cases into one component. That would also allow us to set the margins/padding according to the design, as they look pretty different right now.

Design:

Implementation:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, must've been in a hurry hah

</Text>
<View style={styles.list}>
<SettingItem
name="Use autopilot (recommended)"
Copy link
Contributor

Choose a reason for hiding this comment

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

makes sense 👍

@valentinewallace
Copy link
Contributor Author

Updated @tanx :)

@tanx tanx force-pushed the dev/atpl-choice-setup branch from 440dabf to 95e022a Compare August 21, 2019 12:29
Copy link
Contributor

@tanx tanx left a comment

Choose a reason for hiding this comment

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

@valentinewallace Thanks for the changes. I tweaks some styles but otherwise LGTM 👍

@tanx tanx merged commit 80bb26f into master Aug 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Let users choose if autopilot is enabled during setup

3 participants