sir-lancebot icon indicating copy to clipboard operation
sir-lancebot copied to clipboard

Allow bookmarking a message from a context menu

Open shenanigansd opened this issue 2 years ago • 2 comments

We would like to update the existing bookmark command to also be available from the context menu. #dev-contrib discussion here: https://discord.com/channels/267624335836053506/635950537262759947/1010024284971864104

shenanigansd avatar Aug 19 '22 03:08 shenanigansd

Maybe not necessarily update, but have the regular text-based bookmark command and the context menu one working side-by-side. I think this one will have to wait until #1087 is merged just to avoid merge conflicts or otherwise multiple PRs for the same feature with potentially conflicting code

Robin5605 avatar Aug 19 '22 03:08 Robin5605

This seems fine to me, is the plan to use ephemeral messages when it's invoked or treat it exactly the same as the command invocation? Ephemeral sounds like it would be neater otherwise it would come out of nowhere.

wookie184 avatar Sep 10 '22 15:09 wookie184

I like the ephemeral response route for this.

minalike avatar Dec 21 '22 18:12 minalike

Why isn't this receiving any love? I see the PR you mentioned has been Merged @Robin5605 .

I can yoink this and make our lives better!

shtlrs avatar Dec 21 '22 22:12 shtlrs

@shtlrs you can feel free to work on it but I can be a backup if needed. I've worked on similar code with the eval context menu one and it's quite similar so I should be able to do it as well. So I just want to save you the headache beforehand: you can't have have a context menu decorator inside of a cog. See this comment for more information.

As for making it ephemeral, I'm not sure I like making the resultant method ephemeral because we lose some functionality. Currently, a user can run the !bookmark command, and an embed is sent with a button, just like we want it. Now, other users can also click on this button and get their very own bookmark sent to the same message without having to run the !bm message again themselves. If we make this ephemeral, other users can't do this. I think being able to see that a certain message was bookmarked may hint that said message contains important information to be filed away for later, and provides an easy outlet for other members to see that and get their own bookmark.

Robin5605 avatar Dec 21 '22 22:12 Robin5605

No I definitely see your point concerning the ephemeral part. However, the "definition" of important here is debatable, but also the reach of the message, meaning where it was invoked. If this were to be in py general for example, and that's because a lot of people engage there ( or in other topical channels as well) But this can't always be the case in forum help posts, and having a random message popup could be a bit confusing (just a tiny bit IMO) I struggle to position myself with whether it should ephemeral or not tbh, but I think it wouldn't hurt if it wasn't.

shtlrs avatar Dec 21 '22 22:12 shtlrs

Looks like the general consensus is to keep it ephemeral. If, for some reason, we change out minds at a later date, it's an easy fix. @shtlrs you can go ahead and self-assign now 🙂

Robin5605 avatar Dec 21 '22 23:12 Robin5605

The only reason we added the emoji (now button) was to stop multiple people running the command themselves to bookmark a "good" message and end up spamming the channel with bot responses.

Now that ephemeral messages and context menus exist, I am happy to entirely remove the command portion of it and just have the context menu, as it's far more intuitive and doesn't disrupt the conversation.

ChrisLovering avatar Dec 21 '22 23:12 ChrisLovering

Alright, I'll take this then

shtlrs avatar Dec 22 '22 08:12 shtlrs

@Robin5605 @ChrisLovering @shenanigansd It's sad to see that a user loses the possibility of choosing a title for the bookmark. What do you think about upon interaction, the bot prompts the user for a title, and sends a bookmark for it? I know it may not be the best experience, but I think it's a shame to lose the title feature

shtlrs avatar Dec 23 '22 16:12 shtlrs

It could be a bit annoying for the people that don't use the feature now to suddenly have it be required, but I think that enough of the userbase uses to make it worth implementing.

shenanigansd avatar Dec 23 '22 16:12 shenanigansd

I think the experience for the majority of users who don't often give it a title should be prioritized. We want it to be a simple right click > bookmark > done experience. I'm thinking something like a button attached to the ephemeral embed that opens up a modal allowing you to further edit the title. This way only people who want to have a title for their bookmark embed have to go through this modal and we don't force everyone to. I offer my services for this code as it could be potentially complex and I was also the author of discord.py's modal example so I have some experience working with it. As always, I give first preference if @shtlrs or someone else wants to work on it 🙂

Robin5605 avatar Dec 24 '22 05:12 Robin5605

If we offer the optional ephemeral with modal, how will we time the sending of the DM?

I don't want to sacrifice the ability of adding titles.

minalike avatar Dec 24 '22 06:12 minalike

@Robin5605 Yes that's the original idea. What you said would work just fine. I'm thinking the user gets a text input that comes with 2 buttons, a '' submit '' one and a '' i don't want to give this a title one'' where the whole operation can be skipped. It's just the raw idea of course, and the wording is just there as a placeholder.

@minalike when it comes to timing it, here's how i see it. The user utilizes the context menu, and gets prompted for a title through a view that'll have the previously mentioned components. This prompt will of course have a timeout that can i agree on, I'm thinking something short like a 15 seconds thing. If the interaction timeout is reached, or the user willingly skips it, we send the dm with a default title. Otherwise, we take that input and use it. WDYT?

shtlrs avatar Dec 24 '22 10:12 shtlrs

I don't think having a timeout would be a good idea, someone might take a little longer to type it out and then it would get cancelled.

I do like the idea of a modal with a text input though.

I think this would work best if we had an optional text input, where the placeholder is the title it would be by default.

That way a user can just submit the modal and get the bookmark with the default title, or overwrite it if they want to.

ChrisLovering avatar Dec 24 '22 14:12 ChrisLovering

Well, that's actually a great idea as well.

Especially because the message is ephemeral, so if someone doesn't submit the choice, they just wouldn't get the bookmark, but it also won't flood the channel.

I think I'll go for that, and if anyone else disagrees, we can still discuss it of course.

shtlrs avatar Dec 24 '22 15:12 shtlrs

Looks like I didn't communicate my idea well - So a user would click on the bookmark context menu, and immediately the bookmark would be sent to their DM with the default title. The people who don't care about custom titles are happy - nothing has changed for them.

Now, there is an extra button in the ephemeral message in the channel where it was invoked "click here to change the title". This button brings up a modal where you can enter your title, and on submit, the bot would go back and edit the embed in the DM.

This way the process for regular users is not changed at all and no timeouts are involved

Robin5605 avatar Dec 24 '22 17:12 Robin5605

That seems reasonable

ChrisLovering avatar Dec 24 '22 18:12 ChrisLovering