on3iro / aeons-end-randomizer

🧙🏻‍♀️Awesome Companion App for Aeons End: https://aeons-end-randomizer.de 🧙🏽‍♂️
MIT License
43 stars 24 forks source link

FEATURE: Buttons to deselect cards by keyword #503

Closed Torgen closed 1 year ago

Torgen commented 1 year ago

Fixes #502

on3iro commented 1 year ago

@Torgen This looks great, thanks for the contribution. One minor thing: could you please remove the stray console.logs? :)

Maybe we could even make these toggles instead of deselectors - what do you think?

Torgen commented 1 year ago

I think if some of the cards were selected and some were not, toggling them all would feel weird. I'm not sure what interface would need to do that. Maybe if it required toggling them all in the same direction it could work, but (as in this case) you probably already know what direction that should be.

on3iro commented 1 year ago

Yeah, I would assume that it works like most (de-)select all-checkboxes:

If you have some cards of the respective keyword selected and don't intent to overwrite them, you would just not use the toggle at all.

Torgen commented 1 year ago

Also, I thought you meant the action, not the UI. I don't know if someone who would want to deselect all of a type would ever want to select all of a type. Maybe for Xaxos? I just know the use case my friend (who essentially always uses his browser in incognito mode) has, which is setting up the supply quickly from a blank slate. For that he only cares about deselecting. This only affects supply cards, so deselecting "Fire Token" does carry the risk that you'll get Paradox of Myth and Bone who has several guaranteed cards that add fire tokens.

on3iro commented 1 year ago

Alright, I am fine with that :) Would you mind removing the console.logs? Afterwards we can merge and release this.

Torgen commented 1 year ago

I force pushed with them removed, unless there are some I missed.

On Thu, Jun 15, 2023, 2:33 PM Theo Salzmann @.***> wrote:

Alright, I am fine with that :) Would you mind removing the console.logs? Afterwards we can merge and release this.

— Reply to this email directly, view it on GitHub https://github.com/on3iro/aeons-end-randomizer/pull/503#issuecomment-1593751071, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHRSKCPEVWQWKGA2WWOCDDXLN5SBANCNFSM6AAAAAAZBOEVYI . You are receiving this because you were mentioned.Message ID: @.***>

Torgen commented 1 year ago

Hmm, weird. I force pushed something.

On Thu, Jun 15, 2023, 2:34 PM Dan Rosart @.***> wrote:

I force pushed with them removed, unless there are some I missed.

On Thu, Jun 15, 2023, 2:33 PM Theo Salzmann @.***> wrote:

Alright, I am fine with that :) Would you mind removing the console.logs? Afterwards we can merge and release this.

— Reply to this email directly, view it on GitHub https://github.com/on3iro/aeons-end-randomizer/pull/503#issuecomment-1593751071, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACHRSKCPEVWQWKGA2WWOCDDXLN5SBANCNFSM6AAAAAAZBOEVYI . You are receiving this because you were mentioned.Message ID: @.***>