EdJoPaTo / grammy-inline-menu

Inline Menus for Telegram made simple. Successor of telegraf-inline-menu.
MIT License
354 stars 46 forks source link

Option `setParentMenuAfter` dissapeared in new version #92

Closed esindger closed 4 years ago

esindger commented 4 years ago

Hello, Thank you for the awesome library!

How to return to the parent menu after pressing a button? In v4 we can easily do it by the setParentMenuAfter option. What is properly way to do it now?

EdJoPaTo commented 4 years ago

Thanks for checking v5 out!

When adding a submenu (or chooseIntoSubmenu) you can specify leaveOnChildInteraction. But I missed the setParentMenuAfter too.

https://github.com/EdJoPaTo/telegraf-inline-menu/blob/58da393a817d2fb7fb0f8e0d32eaee45e28407cb/source/buttons/submenu.ts#L6

The idea behind leaveOnChildInteraction are selects with many options or an additional explanation text which you want to show on selecting something. When the user selects something you can go back to the overview. Kind of like drop down menus. It has their use cases but I'm not sure how to deal with cases where we want to go up on only certain buttons.

Thanks for bringing that up, that certainly needs more thoughts

EdJoPaTo commented 4 years ago

In v5 beta 3 I removed leaveOnChildInteraction for simplicity reasons and replaced it by the relative path to go to with the return value of a do function.

When you want to update a menu you can simply return the relative path .. Going to the parent menu can be done with ... As the whole menu is accessible this is kind of powerful but can get confusing for users.

I think this is way better than leaveOnChildInteraction was and allows for more than the "just one layer up parent menu" v4 had with setParentMenuAfter. You can specificy two layers up with ../.. for example.

(The paths are inspired by the unix like change directory command)

esindger commented 4 years ago

This works only for do function but the set callback still has to return void value. Can you fix it please?

EdJoPaTo commented 4 years ago

I thought a bit about the set. I see your point. My basic idea was that toggle / selection should always have a visible feedback to the user -> always update the menu into the same menu afterwards. (Thats one of the basic ideas between choose and select) Only when something is not working (menu hidden) the upper menu should be opened instead. Thinking about that always requiring a string as return value there might be a good thing. It would result in an understanding "hey, there is done something afterwards and you have to specify it". On the other hand it might end up in a lot return '.' statements. Also JavaScript users have no typechecking to hint things like that.

Allowing something without a return value to still refresh the menu would be strange in my opinion as it would differ from the do functions.

Other thoughts about this?

esindger commented 4 years ago

Ok, I got it. Actually, choose menu is suitable because give more freedom. There is only one thing that I really need - to manage what button label to display. In select menu we have isSet option, but here is I have to hard coding active value in the choices argument.

menu.choose('action', () => {
    const choices = {
      foo: 'Foo',
      bar: 'Bar'
    }
    if(choices[ctx.session.selected]) {
      choices[ctx.session.selected] += ' ✅'
    }
    return choices
  }, {
  do: async (ctx, value) => {
    ctx.session.selected = value
  }
})

New option template will be very helpful here:

const choices = {
  foo: 'Foo',
  bar: 'Bar'
}

menu.choose('action', choices, {
  do: async (ctx, value) => {
    ctx.session.selected = value
  },
  template: (buttonKey: string, buttonText: string) => {
    return `${buttonText} ${buttonKey === ctx.session.selected ? '✅' : ''}`
  }
})
esindger commented 4 years ago

Although, this is probably unnecessary, because I can use the buttonText option.

EdJoPaTo commented 4 years ago

select is a more specific case of choose and toggle is a more specific case of interact in a way. Maybe that should be written in some form in the Readme…