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

Add support to directly add MessageEmbeds to Paginator #103

Open
4 of 6 tasks
Andre601 opened this issue Mar 25, 2020 · 6 comments · May be fixed by #104
Open
4 of 6 tasks

Add support to directly add MessageEmbeds to Paginator #103

Andre601 opened this issue Mar 25, 2020 · 6 comments · May be fixed by #104

Comments

@Andre601
Copy link
Contributor

Issue

Issue Checklist

Please follow the following steps before opening this issue.

Issues that do not complete the checklist may be closed without any help.

  • I have checked for similar issues on the issue tracker.
  • I have updated to the latest version of JDA-Utilities.
  • I have checked the branches or the maintainers' PRs for upcoming features/bug fixes.

The issue tracker is reserved for issues, errors, and feature requests related
to JDA-Utilities, and not questions and other requests for help.

Issue Information

Check all that apply:

  • This is a bug report about an error, issue, or bug in JDA-Utilities.
    • I have been able to consistently reproduce this bug.
  • This is a feature request for the JDA-Utilities library.

This issue tracker does not assist or handle issues with the JDA library.

For JDA related issues, visit the JDA issue tracker
and open an issue there.

Description

The paginator, while nice to use, is limited in how much you can customize it.
In fact, other than changing, if the page number should be shown in the footer, the color and the actual description itself is the Paginator not that customizable, considering it using embeds.

My suggestion is to implement support, to directly provide MessageEmbed(s) through a addItems(MessageEmbed...) method.

Why do I not implement it?
The current code of the Paginator is way too strict and complicated, as that it would allow to directly add MessageEmbeds without an entire recode (Unless you guys know a proper way).
I wanted to implement it myself, but it's just way too much and would result on an entire recode on my end, which is not what I want to achieve here, as this would also be a difficult part in a PR.

@jagrosh
Copy link
Member

jagrosh commented Mar 25, 2020

This sounds like it should be a new menu type

@Andre601
Copy link
Contributor Author

I mean sure, but why exactly? I can see many people to want this functionality in the Paginator itself to f.e. show more detailed command-lists with like thumbnails or custom colours...

@jagrosh
Copy link
Member

jagrosh commented Mar 25, 2020

The primary purpose of the paginator is to split up the items into the appropriate-sized lists automatically, provide page numbers, etc.... supplying your own embeds prevents it from doing any of this. If you're supplying your own embeds, there's no paginator functionality anymore, just the general menu functionality of press-reaction->change-view.

@jagrosh
Copy link
Member

jagrosh commented Mar 25, 2020

Providing a custom embed is actually much closer functionality to the Slideshow than to the Paginator

@Andre601
Copy link
Contributor Author

Considering this, did I now try my best to create a EmbedPaginator class for achieving this goal.
I made a PR on my fork so that you guys can take a look.

@Andre601
Copy link
Contributor Author

I made some basic testing with it and it seems to work without errors or issues.

@Andre601 Andre601 linked a pull request Mar 26, 2020 that will close this issue
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants