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

Show recipients alias on header #6944

Merged
merged 7 commits into from
Aug 8, 2019

Conversation

Krist14n
Copy link
Contributor

@Krist14n Krist14n commented Aug 1, 2019

This PR includes the account's alias on the header for the SenderToRecipient component

Copy link
Contributor

@danjm danjm left a comment

Choose a reason for hiding this comment

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

Looks good! Just need to get the build passing.

@danjm danjm changed the base branch from address-book-send to develop August 6, 2019 03:25
@Krist14n Krist14n force-pushed the show-recipients-alias-on-header branch 2 times, most recently from 2520e1b to ce574db Compare August 7, 2019 19:10
@whymarrh whymarrh force-pushed the show-recipients-alias-on-header branch from 24f8316 to 23b0071 Compare August 8, 2019 16:23
@whymarrh
Copy link
Contributor

whymarrh commented Aug 8, 2019

@Krist14n I rebased this on the latest develop and dropped the lockfile changes 😅

@@ -19,6 +19,7 @@ export default class SenderToRecipient extends PureComponent {
senderAddress: PropTypes.string,
recipientName: PropTypes.string,
recipientAddress: PropTypes.string,
recipientNickname: PropTypes.object,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might need to be PropTypes.string:

Suggested change
recipientNickname: PropTypes.object,
recipientNickname: PropTypes.string,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It most definitely should be a PropTypes.string

whymarrh
whymarrh previously approved these changes Aug 8, 2019
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

This LGTM, thanks @Krist14n!

@whymarrh whymarrh force-pushed the show-recipients-alias-on-header branch from b54d02d to 17bc4a5 Compare August 8, 2019 16:52
Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

This LGTM again, QA'd and added 17bc4a5 to fix a small issue. Thanks again @Krist14n! 😅

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.

3 participants