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 confirm dialogue when creating new chat after clicking mail address #3469

Merged
merged 16 commits into from
Nov 6, 2023

Conversation

adzialocha
Copy link
Member

@adzialocha adzialocha commented Nov 3, 2023

This PR adds a confirmation dialogue for the user when an email link was clicked with a) an unknown contact or b) not-yet-existing chat. The behaviour should be the same across email addresses but also mailto: links and resemble the UX flow in the Android app.

deltachat-2023-11-03_15.59.12.mp4

I've also made a mini refactoring moving the mailto-related logic into a separate file and method, trying to decouple it a little bit more from QR functionality.

Potential blocker: Uses unreleased version of the core (compiled from main) including a new get_chat_id_by_contact_id RPC method: deltachat/deltachat-core-rust#4918

Closes #3458

@adzialocha adzialocha changed the title Show confirm dialogue when creating new chat from clicking mail address Show confirm dialogue when creating new chat after clicking mail address Nov 3, 2023
Copy link
Member Author

@adzialocha adzialocha left a comment

Choose a reason for hiding this comment

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

Just adding some comments to explain my decisions to move things around a little, happy to get feedback on naming and placing!

@adzialocha adzialocha marked this pull request as ready for review November 3, 2023 15:11
@link2xt link2xt added the new-core involves or requires an dc-node/core update label Nov 3, 2023
package.json Outdated Show resolved Hide resolved
Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

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

The mailto links of https://github.com/deltachat/interface/blob/master/user-testing/mailto-links.md don't work anymore.

To test either install deltachat from this batch and set as default mailer or easier open the raw md file and copy the full links, then scan qrcode scan from clipboard and also test clicking them from a message

Edit: sorry I probably tested with wrong core.

@r10s
Copy link
Member

r10s commented Nov 4, 2023

hi, @adzialocha! first of all, welcome aboard and congrats for landing your first prs!

in general, thanks a lot for the great pr description and for the nice video, giving ppl a bit outside desktop code a good overview what a pr is about. this is very welcome!

Copy link
Member

@Simon-Laux Simon-Laux left a comment

Choose a reason for hiding this comment

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

Just tested with the core master, works now as expected for me. Sorry for the confusion.
We can merge after you addressed my review comments and we have the new core release with the new jsonrpc api.

BTW: maybe we should have a different error when the chat stuff failed, "mailto parse failed" is rather confusing.

* master:
  fix window store installation (remove unknown language code from supported languages)
  Update translations (05.11.2023)
  remove the `any`, as electron types the `request` parameter already in this callback
  fix tests
  fix #3470
  remove useless if statement
  add CHANGELOG entry
  run `npm run check:format`
  shorten 'Forward' and move it near 'Reply' where it belongs to
  reword to 'Encryption Info'
  reword profile title to 'Profile'
  reword to 'Switch Account'
  rare errors are not translated
  reword to 'Export Backup'
  add changelog entry
  use correct autocrypt-setup strings
  Update translations (02.11.2023)
  Add entry to CHANGELOG.md
  Correctly center username next to profile image
@adzialocha
Copy link
Member Author

Thank you for your review comments and confirming that all is good with the links now @Simon-Laux!

I like the idea to have an own helper method for the confirmation dialogue, would tackle that in a separate, follow up PR if that makes sense?

Also, I assume we will not merge this until the new core is released, right?

@link2xt
Copy link
Collaborator

link2xt commented Nov 6, 2023

Also, I assume we will not merge this until the new core is released, right?

New core 1.129.0 is already released.

@Simon-Laux Simon-Laux merged commit 8e7eea1 into master Nov 6, 2023
1 of 5 checks passed
@Simon-Laux Simon-Laux deleted the confirm-new-chat branch November 6, 2023 16:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new-core involves or requires an dc-node/core update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

clicking email address creates chat immediately
4 participants