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

instant success on simple qr-setup-contact #1215

Merged
merged 2 commits into from
Jan 26, 2020
Merged

instant success on simple qr-setup-contact #1215

merged 2 commits into from
Jan 26, 2020

Conversation

r10s
Copy link
Contributor

@r10s r10s commented Jan 24, 2020

this pr changes the behavior when scanning a setup-contact qr-code.
instead of waiting, until the whole protocol is finished,
which may take something between 10 seconds and several minutes,
the dc_join_secure_join() returns instantly;
the uis typically show the created chat then.

the returned chat-id is the same than if we wait for the protocol to finish,
it is the opportunistic one-to-one chat-id, so no changes there.

all this works even when both devices are offline.

after dc_join_secure_join() returns, however,
the usual setup-contact continues. ux-wise, once the protocol finishes,
a system-info-messages is added to the chat (also unchanged),
this is directly visible, in the chat as well as in the chatlist.
while the prococol runs, the user can alredy send message to the chat,
or do other things in the app.

if the user scans a new qr-code while an existing protocol is not finished yet,
the old join will be aborted (however, of course, created chats are kept).
we could also allow multiple joins at the same time,
however, this would be much more effort that this little change
and i am not sure if it is worth the effort.

finally, if a verified-group shall be joined,
this is not possible instantly, this is not affected by this pr.
same for unverified-groups, however, this could maybe be improved,
but also here, not sure if it is worth the effort
(i think most scans are setup-contact scans)

changes in the ui are not required,
i tested this a bit and it seems to work as expected,
not sure, if i've overseen something,
i think it is a HUGE win for the whole setup-contact thingie :)

@Jikstra
Copy link
Contributor

Jikstra commented Jan 25, 2020

Very huge improvement! Thanks :)

Copy link
Contributor

@flub flub left a comment

Choose a reason for hiding this comment

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

Could you add a doc comment on the function describing it's high-level functionality and behaviour rather than bury it within normal comments nested in if-else branches?

@flub
Copy link
Contributor

flub commented Jan 25, 2020

Reminder that https://doc.rust-lang.org/rust-by-example/meta/doc.html is a great tl;dr for doc comments.

@r10s
Copy link
Contributor Author

r10s commented Jan 25, 2020

Could you add a doc comment on the function describing it's high-level functionality and behaviour rather than bury it within normal comments nested in if-else branches?

the high-level overview is at dc_join_securejoin() which is the direct ffi-counterpart. but yes, maybe we can say at least that and drop a one-line-overview. and yes, also the description there needs some adaption wrt this pr. i'll add a commit.

r10s added 2 commits January 25, 2020 22:22
this changes the behavior when scanning a setup-contact qr-code.
instead of waiting, until the whole protocol is finished,
which may take something between 10 seconds and several minutes,
the dc_join_secure_join() returns instantly;
the uis typically show the created chat then.

the returned chat-id is the same than if we wait for the protocol to finish,
it is the opportunistic one-to-one chat-id, so no changes there.

all this works even when both devices are offline.

after dc_join_secure_join() returns, however,
the usual setup-contact continues. ux-wise, once the protocol finishes,
a system-info-messages is added to the chat (also unchanged),
this is directly visible, in the chat as well as in the chatlist.
while the prococol runs, the user can alredy send message to the chat,
or do other things in the app.

if the user scans a new qr-code while an existing protocol is not finished yet,
the old join will be aborted (however, of course, created chats are kept).
we could also allow multiple joins at the same time,
however, this would be much more effort that this little change
and i am not sure if it is worth the effort.

finally, if a verified-group shall be joined,
this is not possible instantly, this is not affected by this pr.
same for unverified-groups, however, this could maybe be improved,
but also here, not sure if it is worth the effort
(i think most scans are setup-contact scans)
@hpk42 hpk42 merged commit 49fdd6f into master Jan 26, 2020
@hpk42 hpk42 deleted the instant-qr-scan branch January 26, 2020 15:23
@r10s r10s mentioned this pull request Oct 29, 2021
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.

4 participants