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

Modals: Make Unsafe Less Useful #345

Open
gdude2002 opened this issue Nov 9, 2024 · 1 comment
Open

Modals: Make Unsafe Less Useful #345

gdude2002 opened this issue Nov 9, 2024 · 1 comment
Assignees
Labels
Status: Blocked Waiting on something else to be ready. Type: Enhancement Improvements to existing features.

Comments

@gdude2002
Copy link
Member

While chatting with @AugustoMegener, I had an idea regarding how Modals are currently implemented - why can't each type that supports modals support passing their relevant data to the Modal constructor?

Like, if we look at the String-based select menu types, EphemeralStringSelectMenu takes (() -> M)? as the Modal builder, but I don't see why it couldn't take, for example, ((List<String>) -> M)?.

I think we could implement this with minimal friction, and I also think we could do it without breaking the current API. However, I'm currently working on something else, so I've created this reminder issue. I'll look into this later on!

@gdude2002 gdude2002 added the Type: Enhancement Improvements to existing features. label Nov 9, 2024
@gdude2002 gdude2002 self-assigned this Nov 9, 2024
@gdude2002
Copy link
Member Author

I've been looking into doing this, and while I don't mind the busywork behind it, I've realised that it's difficult to do this reasonably because of how Discord works.

Essentially, all interaction-based contexts contain the initial interaction response. This is an issue because a Modal must be sent as the first interaction response, meaning a component cannot be created before we attempt to send a Modal. As the contexts are responsible for converting component values to rich types, this creates a circular dependency that can't easily be fixed without significantly complicating the API and worsening the UX.

This is a bit simpler for commands, which have their arguments parsed before the context is created. However, I don't want to implement a Modal API for commands that isn't consistent with the rest of the Modal APIs.

As a result, I'll be shelving this idea for now, at least until I can think of a better approach.

@gdude2002 gdude2002 added the Status: Blocked Waiting on something else to be ready. label Nov 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Blocked Waiting on something else to be ready. Type: Enhancement Improvements to existing features.
Projects
None yet
Development

No branches or pull requests

1 participant