search

FlowCrypt / flowcrypt-browser

FlowCrypt Browser extension for Chrome and Firefox
https://flowcrypt.com
Other
373 stars 46 forks source link

bcc and cc #1065

Closed chuacw closed 5 years ago

chuacw commented 6 years ago

Is there no BCC, and CC available when sending emails?

tomholub commented 6 years ago

Not yet. I have a list of users who are interested in this, I'll leave this open to track the progress. Thanks.

tomholub commented 5 years ago

customer who wants this https://mail.google.com/mail/u/0/#sent/FMfcgxwBTjvMwPmdNCmSVkvHtRwndRph

tomholub commented 5 years ago

This is a relatively UI-heavy change. The result should work very similarly to what our Android app does with cc/bcc.

tomholub commented 5 years ago

This should also support drag and drop between to:, cc: and bcc:, else it's hard to use.

chuacw commented 5 years ago

That's true. The program model and user model needs to match here.

tomholub commented 5 years ago

This is a relatively large change, and will need thorough tests added.

michael-volynets commented 5 years ago

@tomholub Can you please share the screenshot of Flowcrypt Android App where I can see BCC/CC?

tomholub commented 5 years ago

I recommend to download the app and test it out (do you have an android phone?) so that you can mimic the whole interaction. There is a down chevron on the right side next to recipient, which you can click to view cc and bcc options.

tomholub commented 5 years ago

image

Can otherwise follow Gmail

michael-volynets commented 5 years ago

In this issue, I will also refactor composer-contacts.ts to make it easier to understand code and to make the logic with CC/BCC similar to to, (perhaps I will have to delete parserenderRecipients and rewrite or split it)

tomholub commented 5 years ago

Of course. Is it possible to refactor it in a separate PR with no functionality change? I know it will be more work for you, but it will be much easier for me to review, and therefore less likely that a mistake will slip through.

michael-volynets commented 5 years ago

Yes, sure,

Okay, I will try to refactor it in separate PR

tomholub commented 5 years ago

Great. Let's do the refactor first, then you'll add new functionality after that.

michael-volynets commented 5 years ago

@tomholub, will we allow users to add the same email address to the to/cc/bcc if the email was already added in another input? I checked how Gmail does it and they allow to do it but they even allow to add the same email addresses in one input.

In my opinion, it doesn't make sense to allow doing it but there maybe there are some situations that I don't know where it will be useful

tomholub commented 5 years ago

I'm not sure, doesn't make much sense to me either. I wonder if a total noob user would go bothering Google that "their recipient won't add" when they already added him/her 3 times. So maybe it's a UX thing. Hard to say, but for now I'd say don't add any special logic (things that add more code) to handle recipients duplicate in cc/bcc. It's not a big deal.

michael-volynets commented 5 years ago

CC is working now but I've got a problem with BCC. We are using an old version of emailjs-mime-builder and to make it work I need to update it to the newer one.

I will create another PR for that

tomholub commented 5 years ago

We use a fork of emailjs, basically. I made some changes to it. So it may be a bit more difficult than that. What issues does the old one have bcc in particular?

michael-volynets commented 5 years ago

In the newer version, they accept this option includeBccInHeader (https://github.com/emailjs/emailjs-mime-builder/blob/master/dist/builder.js#L98) and we don't have this option because we use an older version so we always skip bcc here: https://github.com/FlowCrypt/flowcrypt-browser/blob/master/extension/lib/emailjs/emailjs-mime-builder.js#L399

Could you please give me access to the forked repository to make this change and update our emailjs-mime-builder.js

michael-volynets commented 5 years ago

I think we can even remove that case

tomholub commented 5 years ago

It's not exactly a forked repository, I simply edited the source files in place :blush: ... I think, if you can, please ship this feature without BCC first. I think I will need to be involved in updating emailjs, which I planned to do for a long time anyway. But not right now, my life is a bit hectic atm

michael-volynets commented 5 years ago

Sure, okay I will change it in our file

tomholub commented 5 years ago

Finally merged the refactor. So again, please let's ship this as CC option only. Leave BCC for another day, after I upgrade emailjs.

michael-volynets commented 5 years ago

Why? I think if you have already edited emailjs, so I can edit it too, according to the current version, there are only about 5-6 lines of code I need to add to make BCC working

tomholub commented 5 years ago

Ok, if it is a very small change, we could do that.