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

Refactor verification request code #1109

Merged
merged 7 commits into from
Dec 10, 2019

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Dec 9, 2019

Part of element-hq/element-web#11224

This extracts most of the verification request code from crypto/index and puts it into it's own VerificationRequest class with the specificities of whether to use to_device verification or verification over DM into a Channel class. Care has been taken to maintain the same public API from the crypto module and the request object passed as an argument of the crypto.verification.request event.

The next step will be add support for the m.key.verification.ready event, and to decorate verification events with a verificationRequest property and use this to drive toasts and verification timeline tiles in the react-sdk, removing the KeyVerificationStateObserver.

@bwindels bwindels requested a review from a team December 9, 2019 17:06
@bwindels bwindels removed their assignment Dec 10, 2019
@bwindels
Copy link
Contributor Author

(cleared myself as assignee to avoid misunderstandings that this is somehow waiting for me to do anything; it's not, and fully ready for review)

@dbkr dbkr requested review from dbkr and removed request for a team December 10, 2019 10:17
Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

Massive thanks for moving things out of crypto/index.js! This looks much nicer.

@bwindels bwindels merged commit 817bfa3 into develop Dec 10, 2019
@t3chguy t3chguy deleted the bwindels/accept-verif-request-rebased branch May 10, 2022 14:27
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