Add/remove recipient from compose

[lirikooda]

See also #148 I'd love to add unit tests, but would need some pointers for that please.

[josteink]

Hey there.

While the changes here don’t look wrong per se, and the TypeScript definitions are definitely improved, it seems like the new functions added are kind of overlapping with what the gmail.dom.email.{to,cc,bcc} functions are supposed to provide.

Are there any reason those functions don’t work for you as is?

[lirikooda]

I agree that it'd be better to have the add/remove functionality work with the current getter api. However, it simply doesn't work as is. See the relevant issue: https://github.com/KartikTalwar/gmail.js/issues/148 I could reopen it and expand there. In any case, the returned value from {to, cc, bcc} is simply not enough to make the functionality work. Especially "remove" which is unrelated to the textarea at all.

[josteink]

Right. But if they don’t work as is, wouldn’t it make more sense to fix them in place, rather than adding new implementations in new functions?

I agree we lack a good/consistent remove-api though.

On a side-note (it’s early Sunday here) why are modifying api.dom.email instances anyway? They’ve long been sent and any change we do is purely in the UI of the user, right? 🙂

[lirikooda]

I think fixing in place is kinda problematic. At the moment, these methods accept a single string, you'd expect a string[] if you want to set entire recipient field. And I wouldn't want to break functionality. Adding ui changes is very useful, having it as part of the library is an asset IMO. In any case, I'm more than happy to align with the way we'd want the apis to be defined, let's just keep in mind all the use cases.

[josteink]

Sorry about the slow response here. Work has taken priority lately :)

So to recap:

• The PR title says add/remove from compose
• The code says add/remove from email
• This has caused a little confusion for this poor maintainer as he is struggling with context-switches :smile:
• We already have some methods, at least for adding addresses to emails.
• But you don't want to risk breaking those by adding support for your use-case.

That's about correct?

I appreciate how you want to avoid breaking existing code, but I'd also don't want to make the API difficult to navigate by having several similar, but not quite, methods for doing kinda the same stuff, but in different ways. Not to mention the code-duplication that might lead to. So yeah.

In general, I there's no problem making a method kinda polymorphic. In fact we do that all over in gmailjs already :grin:

For instance gmail.dom.email.to() accepts both arrays and non-array inputs. There's no gmail.dom.email.cc() or gmail.dom.email.bcc(), but I think adding them would make the API more at parity with the gmail.dom.compose-class, so that would be a good thing.

So if that's ok, that would leave the only thing remaining to be removing recipients.

That can be solved through a seperate method, or an optional bool like

    const email = gmail.dom.email();
    email.to({name: "John Doe", email: "john.doe@gmail.com"});
    const remove = true;
    email.to({name: "John Doe", email: "john.doe@gmail.com"}, remove);

How about that?

[lirikooda]

Sounds good. Sorry I wasn't too familiar with the difference between compose/email contexts. I'll work on adjusting the api and will reopen the PR then.

[josteink]

Sounds good. Let me know if you want or need any more feedback 🙂