python-discord
Add ability to view previous topics with .topic command
Relevant Issues
Closes #1145
Description
After a user invokes the ".topic" command and reacts with the reroll emoji, the embed will now show the previous few conversation prompts. It cannot display more than 256 characters because of the limit on embed title character length, which is usually around 5 prompts.
Did you:
- [x] Join the Python Discord Community?
- [x] Read all the comments in this template?
- [x] Ensure there is an issue open, or link relevant discord discussions?
- [x] Read and agree to the contributing guidelines?
I'll keep this open, but be sure next time to get your issue approved by a core dev before continuing with a PR.
Could you share a screenshot of what this looks like after being used a few times?
I think we will also need to add a limit to the number of previous topics that get shown, if there isn't already present. Users could easily spam the button and cause a very large embed from the bot otherwise.
Discord sets the limit on the number of characters in the title of an embed to be 256. From my testing, that seems to mean there's around 4 or 5 topics that can be on display at once. Personally I think 4 or 5 is enough for people to have something to talk about, so it seems good to me.
Here's some screenshots of my bot running the code
I think a better way to deal with this would be when a user clicks on the emoji the new topic becomes number 1 and the rest get pushed down a number.
I considered this,but this would change the ordering of the older messages. One point of adding this feature is so that people wanting to respond to a specific topic could include the number of that topic in with their reply, e.g. someone responding to the second conversation starter could say something like "For topic 2, ...". Changing the numbers could be confusing or change the sentence someone was referring to.
I think a better way to deal with this would be when a user clicks on the emoji the new topic becomes number 1 and the rest get pushed down a number.
I considered this,but this would change the ordering of the older messages. One point of adding this feature is so that people wanting to respond to a specific topic could include the number of that topic in with their reply, e.g. someone responding to the second conversation starter could say something like "For topic 2, ...". Changing the numbers could be confusing or change the sentence someone was referring to.
If we're going to go with that behaviour, we should remove the reaction button when it will no longer send a new topic.
This seems good to me, the only thing I would suggest is limiting the number of prompts to 3, as 5 seems like quite a lot (i'd still leave the detection for when it goes over the title limit, just in case there are 3 really long topics).
Let's see what others think first though, as I don't mind too much and some people may prefer more.
Could you move the previous topics as part of the description instead? I feel like it might be too distracting to be in all bold and big like that
Could you move the previous topics as part of the description instead? I feel like it might be too distracting to be in all bold and big like that
We could do that, but there's a few things to work out first. What would the title in bold be? And what would we set the limit for the topics to show as, since we wouldn't be bound by the title character limit?
Hey @Anonymous4045, are you still planning to work on this?
I agree with wookie that allowing five topics simultaneously is a little too much, perhaps 3 could do. If a topic is approved in the repo, they should already be considered plausible topics. Keep in mind that people also like to reference the topic embed when replying to the topic. Having too many topics to choose from in that embed can lead to fragmented conversations, which one might say defeats part of the purpose of this .topic command in the first place.
Which also means, regarding Chris' suggestion to pop the first topic off and append new ones to the bottom: Breaking the topic number reference is a problem, like you said. So removing the ability to reroll altogether on the third[^1] time the emoji is reacted seems good enough.
This addresses both Chris' concern (with reaction not actually getting a new topic) and yours (people responding to Topic N).
[^1]: Or whatever the decided limit is.
What would the title in bold be?
Since titles are not mandatory, moving the topic from the title to the description could look better once the reroll emoji is reacted IMO.
So all that this PR needs now as far as I can tell is to 1) improve the UI (bold text) and 2) implement a reasonable behaviour to limit the number of topic rerolls allowed. What do you think?
@Xithrius thanks for the reminder! I've implemented most of what you said. I also tried switching the suggestion form to be a footer, but this seems to break the hyperlink. Do you know of a way to have links in the footer? We could also just make it in the description, but I think it looks better as a footer for formatting purposes.
Thank you for the update!
Footers in embeds do not support hyperlinks, unfortunately. I believe previously the link was just a part of the description.
Looks good to me! I'll test out this PR on my local machine in the coming days.