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

feat: Keyboard button builders and Keyboard.from #104

Closed
Tracked by #202
KnorpelSenf opened this issue Nov 23, 2021 · 12 comments · Fixed by #420
Closed
Tracked by #202

feat: Keyboard button builders and Keyboard.from #104

KnorpelSenf opened this issue Nov 23, 2021 · 12 comments · Fixed by #420
Labels
good first issue Good for newcomers
Projects

Comments

@KnorpelSenf
Copy link
Member

KnorpelSenf commented Nov 23, 2021

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

@KnorpelSenf KnorpelSenf added the good first issue Good for newcomers label Nov 23, 2021
@KnorpelSenf KnorpelSenf changed the title Keyboard button builders and Keyboard.from feat: Keyboard button builders and Keyboard.from Nov 23, 2021
@EdJoPaTo
Copy link
Member

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
Copy link
Contributor

Loskir commented Nov 29, 2021

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
Copy link
Member Author

KnorpelSenf commented Nov 29, 2021

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
Copy link
Member

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
Copy link
Member Author

KnorpelSenf commented Nov 29, 2021

I think the idea is to be able to do

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

@Loskir
Copy link
Contributor

Loskir commented Oct 21, 2022

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
Copy link
Member Author

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

@KnorpelSenf
Copy link
Member Author

KnorpelSenf commented May 31, 2023

I'm looking forward to things like

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

to let the user pick a month

@Loskir
Copy link
Contributor

Loskir commented May 31, 2023

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

@KnorpelSenf
Copy link
Member Author

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
Copy link
Member

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
Copy link
Member Author

KnorpelSenf commented Jun 1, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
Status: Done
Coding
To do
Development

Successfully merging a pull request may close this issue.

4 participants