grammyjs / grammY

The Telegram Bot Framework.
https://grammy.dev
MIT License
2.21k stars 110 forks source link

feat: Keyboard button builders and `Keyboard.from` #104

Closed KnorpelSenf closed 1 year ago

KnorpelSenf commented 2 years ago

Currently, we can only create complete keyboards using the keyboard plugin. It may be useful to add helpers that create individual buttons.

We should then add a static method Keyboard.from which takes a 2D-array of button objects, and creates a keyboard from them. Optionally, it handles reshapes on the fly. As a result, you can also pass a 1D-array and a number of columns/rows and the created keyboard will have the specified dimensions.

All of the above should be done for inline keyboards too.

Requested by @Loskir in the discussion around https://t.me/grammyjs/11332

EdJoPaTo commented 2 years ago

Putting n buttons into m columns is something helpful that should be part of the main library in my opinion.

See grammy-inline-menu align.ts and its test for inspiration.

Creating buttons via functions seems overkill to me as its literally only an object with two keys which are autosuggested from typings (even for JavaScript users). This would only create pointless boilerplate code which abstracts something that isnt worth abstracting.

Loskir commented 2 years ago

Creating buttons via functions seems overkill to me as its literally only an object with two keys which are autosuggested from typings

I disagree. Having to write text and callback_data every time is ridiculous

KnorpelSenf commented 2 years ago

I am not sure if I'd ever use the button builders myself, but I do see that especially in functional patters such as map and reduce it can be very helpful to have util functions for this. Also, given that there seems to be demand for it, I really don't mind adding it. Telegraf can do it, too, and there's virtually no reason against supporting it, except that we have six lines of code more in the library.

EdJoPaTo commented 2 years ago

I don't see the need in functional patterns which I use all the time. I also don't see the argument of writing callback_data which is auto suggested and auto completed from IDEs.

const buttons = ['a', 'b'].map(o => ({name: o, callback_data: `something-${o}`}));
KnorpelSenf commented 2 years ago

I think the idea is to be able to do

const keyboard = Keyboard.from(['a', 'b'].map(o => Keyboord.button(o, `something-${o}`)));
Loskir commented 1 year ago

Update: this is already partially implemented, we have new Keyboard([[buttons]])

Helper functions for buttons are not here yet, but we have a package (https://github.com/loskir/grammy-markup) that provides them.

new InlineKeyboard([
  [IButton.text("Get random music", "random")],
  [IButton.switchInline("Send music to friends")],
])

I hope we're still planning to add this feature in one form or another. For example, we can add static methods on Keyboard and InlineKeyboard so they can be used like InlineKeyboard.textButton('a', 'b'). However, it's not very compact

KnorpelSenf commented 1 year ago

I agree that we should add static methods to the keyboard classes. I'm comparably unsure about the naming, though …

KnorpelSenf commented 1 year ago

I'm looking forward to things like

Keyboard.from([MONTHS]).reflow()

to let the user pick a month

Loskir commented 1 year ago

Why not Keyboard.from(chunk(MONTHS, 4)) after all? Why create an intermediate keyboard that does nothing?

KnorpelSenf commented 1 year ago

That will obviously work if you prefer to work on raw arrays. I don't. I'd like to tell the keyboard how many columns it should have, and then it will do that. I don't want to know about transforming arrays, neither with nor without a third-party library.

EdJoPaTo commented 1 year ago

As soon as you are using buttons of multiple things at the same time like months and years you need to put both into columns and not mix them together. Also at the end should be a full width confirm button. In my opinion it’s needed to work with the double array structure to get the keyboard you want. Simplifying the whole menu with a simple array with a column width setting will not work for most cases.

KnorpelSenf commented 1 year ago

While correct, this is unrelated from the current discussion. Obviously we support custom layouts with granular control.

The point is that if this is not needed (the array can be processed by chunk) then the exact array structure does not matter to me (by definition) and so I also don't want to work with the array. This is about "if I can use a higher-level API, I want to not care about the details anymore" rather than a question of when automatic layouting works.