✨ Button support - Githubissues

Nico105 commented 1 year ago

Changes

Adds an option in the default manager or start() options to provide a join and optionally a leave button. (if only join is provided, it will also act as leave, if clicked twice)

Usage test

client.giveawaysManager.start(giveawayChannel, {
   buttons: {
            join: new Discord.ButtonBuilder()
                .setLabel('Join')
                .setStyle(Discord.ButtonStyle.Primary)
                .setCustomId('join'),
            leave: new Discord.ButtonBuilder() // Optional as mentioned
                .setLabel('Leave')
                .setStyle(Discord.ButtonStyle.Secondary)
                .setCustomId('leave')
        }
}

Questions

~~What should happen with endedGiveawayReactionAdded ? It was added because some people want to prevent users from joining in case of a reroll. This one would a little bit be the odd one out since it is reaction only, but emitting it for buttons too doesn't really make sense cause of its specific use case. LMK~~

~~If there should be a 3rd option which takes an array for any other buttons that should get added to the message and should get checked/filled in. Tho I have to say thats not a high priority cause that could get added later anyway. LMK~~

~~The buttons are currently called join and leave. I leave it up for debate if they should be called differently, like instead of join --> enter or smth. LMK~~

~~I currently just named the button events giveawayJoined and giveawayLeft cause the reaction ones wouldn't really fit. But so I would like to ask if we should rename the reaction ones to these names too, cause the only event argument difference is messageReaction vs ButtonInteraction, which can be detected easily via if (giveaway.buttons). What i also leave open is the event names themselves, it doesn't have to be those I used on a whim. LMK~~

~~Important: Currently the interactions are not deferred, so they just fail. I did this so the dev can decide in the events on what to do. But lmk if we should do something by default~~

Stuff to do

[X] ~~Proper priority handling of reaction vs button. Cause it is possible to provide both as the default manager option and then you could set one or the other as null in the start() options to indicate which default should be used. But also atm i left it open what to do when a button is provided but it is invalid. Should it automatically default to the reaction and start or reject, or whatever. Vice versa. Note: I left some time pass by, so Im not remembering the current behavior/priority stuff completely = the paragraph above is probably worded wrong/describes it falsly but Im too lazy atm to check it again. Still this has to get properly handled.~~ Priority (if I'm not mistaken): start()-reaction > start()-buttons > default-reaction > default-buttons
[x] Add proper drop compatibility.
[X] Document everything properly.
[x] Better README description (I guess).

Status

Tested:

Basic functionality
Event emitting
Detecting button changes and editing them.
Drop compatibility.
[x] These changes have been tested and formatted properly.
[X] This PR introduces some breaking changes.

reinacchi commented 1 year ago

This looks absolutely amazing. Will definitely test this 👍🏻

Nico105 commented 1 year ago

@Androz2091, could you maybe take a look at the questions stated above

Nitsugua38 commented 1 year ago

Hi, is it fully operational now or not yet ?

SuperDzej commented 1 year ago

Think it should add also others entry as component array, so user can add some info buttons they need like link to some page for giveaways, or similar buttons that they find useful?

AmNobCop commented 1 year ago

Is this PR complete?

1887jonas commented 1 year ago

Nice idea, but if a different database than json file is used to store the giveaways, it doesn't work yet

Nico105 commented 1 year ago

Nice idea, but if a different database than json file is used to store the giveaways, it doesn't work yet

will be fixed

Nico105 commented 1 year ago

Is this PR complete?

not yet, last changes have to be made

AmNobCop commented 1 year ago

Is this PR complete?

not yet, last changes have to be made

Great, thank you for the hard work you are putting into it, will it show the number of entries on the button?

Nico105 commented 1 year ago

Is this PR complete?

not yet, last changes have to be made

Great, thank you for the hard work you are putting into it, will it show the number of entries on the button?

it will be possible to do that

AmNobCop commented 1 year ago

Have you finished it yet?

TejasLamba2006 commented 1 year ago

I have made modifications to the discord-giveaways forked by @Nico105 and have been successful in creating my own working button giveaways package. However, I am unsure of the potential effects of merging my changes back into the original package i.e discord-giveaways, so I have decided to create a separate NPM package for my modified version. Is it appropriate for me to do so?

twlite commented 1 year ago

I have made modifications to the discord-giveaways forked by @Nico105 and have been successful in creating my own working button giveaways package. However, I am unsure of the potential effects of merging my changes back into the original package i.e discord-giveaways, so I have decided to create a separate NPM package for my modified version. Is it appropriate for me to do so?

Yeah it is, but consider giving credits to the original one.

TejasLamba2006 commented 1 year ago

@skdhg, This would be fine I guess