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

Generalize set account name popup #13603

Merged
merged 37 commits into from Sep 12, 2018

Conversation

nathanmsmith
Copy link
Contributor

This moves the "Name Account" popup into wallets/common and uses the WalletPopup component.

I'm working on another PR that uses the "Name Account" popup, so I thought I'd start by generalizing the component out of the LinkExisting screen.

@nathanmsmith nathanmsmith changed the title [WIP] Generalize set account name Generalize set account name popup Sep 6, 2018
@nathanmsmith
Copy link
Contributor Author

Note also this PR is based off of #13595, so I'd appreciate if you could review that too!

@nathanmsmith
Copy link
Contributor Author

@keybase/react-hackers

@nathanmsmith
Copy link
Contributor Author

bump

@nathanmsmith nathanmsmith requested a review from a team September 7, 2018 19:47
@@ -10,7 +10,7 @@ const account = {
}

const load = () => {
Sb.storiesOf('Wallets/Common/Participants Row', module)
Sb.storiesOf('Wallets/Common/Account Entry', module)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Think I might have been doing a little too much copy-pasting when I did this the first time

</Kb.Box2>
)}
<Kb.Box2 direction="vertical" style={styles.outerContainer}>
{props.onBack && <Kb.HeaderHocHeader onBack={props.onBack} headerStyle={styles.header} />}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Having an onBack prop in WalletPopup will be useful in cases such as closing the create new account and link existing account popups when they're opened from the send form.

@@ -18,7 +18,6 @@ const common = {
onDone: action('onDone'),
onKeyChange: action('onKeyChange'),
onNameChange: action('onNameChange'),
onViewChange: action('onViewChange'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

onViewChange wasn't being called anywhere in the LinkExisting component; it wasn't even defined in its props.

@nathanmsmith
Copy link
Contributor Author

I also added the autofocus props to the inputs in this PR, so this takes care of DESKTOP-7727

const load = () => {
Sb.storiesOf('Wallets/Common/Enter Name Popup', module)
.add('Enter name', () => <EnterNamePopup {...enterNameProps} />)
.add('Prefilled name', () => <EnterNamePopup {...enterNameProps} name="mikem's third account" />)
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well remove this + Name error from Wallets/Link existing since they're nearly dupes

@nathanmsmith nathanmsmith merged commit 700b427 into master Sep 12, 2018
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.

None yet

2 participants