-
Notifications
You must be signed in to change notification settings - Fork 731
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
Use different copy for self verification #3624
Conversation
The copy all refers to 'their' device for when you're verifying someone else. Update it to refer to your own devices in the case where you're verifying one of your own devices.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks.
One remark: the "Can't scan" option is also a bit weird when self-verifying :/. We should maybe hide the subtitle in this case?
oh, good point |
and tweak the other string more because it's not "each other" either
updated |
var scanOtherCodeTitle: String | ||
var compareEmojiSubtitle: String | ||
if (state.isMe) { | ||
scanCodeInstructions = host.stringProvider.getString(R.string.verification_scan_self_notice) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I did not noticed it before, but there is no need to use host.
here. It only useful if you want to use the host member from the Epoxy item DSL, for instance inside bottomSheetVerificationNoticeItem {}
block.
It's not a big deal at all, it's OK to merge the PR like that, I can do the clean up later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Fixed in 07d6eaa)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah - I see, I did wonder what that was all about, but figured it was best to be consistent anyhow
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the update, LGTM
The copy all refers to 'their' device for when you're verifying
someone else. Update it to refer to your own devices in the case
where you're verifying one of your own devices.
Pull Request Checklist
Equivalent for iOS: element-hq/element-ios#4525