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

Move adminui.Frame (and the ability to use adminui.Menu) to the MultiBot dispatcher #66

Open
nmlorg opened this issue Jul 20, 2019 · 3 comments
Assignees
Labels
cleanup Code changes that improve maintainability without changing behavior

Comments

@nmlorg
Copy link
Owner

nmlorg commented Jul 20, 2019

Breaking this out from #63-507371095, the first step might be to add a new parent field to adminui.Menu.fields[]. If set to None, Menu.dispatch will continue to use frame.value as the subframe's parent; otherwise, it'll use field.parent (as it were). This will allow menus to jump contexts easily—so if the default context for a top-level dispatcher is ctx.userinfo, so for example events.settings can naturally continue with the natural path, moderator.moddispatch can still switch into ctx.groupinfo when jumping to moderator.admin.

This might also require Menu.select(..., create=NEWHANDLER) to actually be replaced with something like Menu.add(field=None, handler=NEWHANDLER, parent=NEWPARENT) (just to allow both handler and parent to be specified).

@nmlorg nmlorg added the cleanup Code changes that improve maintainability without changing behavior label Jul 20, 2019
@nmlorg nmlorg self-assigned this Jul 20, 2019
@nmlorg
Copy link
Owner Author

nmlorg commented Aug 12, 2019

One minor issue is the separation of the actual versus presentational label for each frame. Right now, when you enter /admin, it displays its name as "Bot Admin" in the breadcrumbs (from msg.path), and /mod is "Group Admin", etc.  When I went to convert /admin into using a new top-level adminui.Menu for its dispatch, nearly every test case had a diff because the "Bot Admin" necessarily changed to "/admin" (or just "admin").

I'm already changing Menu.add (and Menu.fields) to include a parent field to allow handlers to jump contexts, but it might be necessary to split the description of the current frame further:  A literal name (the entry in parent it's associated with, and also the canonical name to include in msg.path for callbacks), a free-form description (used in Menu.display), and a new display name (to override the literal name in Menu.display's button list as well as to appear in msg.path's breadcrumbs).

While I'm here, I think it might be worth moving the field-sorting logic into Menu.add, if only because I'm eliminating create=True in favor of menu.add(None) and the simplest implementation (in Menu.select) would be to just assume fieldname is None means to catch any field during the loop (rather than setting it aside and duplicating some logic outside the loop).

@nmlorg
Copy link
Owner Author

nmlorg commented Aug 12, 2019

Also, just to document my current thinking, I'm going to create an adminui.Menu in the MultiBot dispatcher, then during the pre-dispatch phase all modules will be provided the ctx and menu and be expected to register their entry points in the latter (optionally after examining the former, to for example not register /admin if the user isn't an admin in any bots). help will probably need access to that top-level Menu, so it will either need to be passed along with the frame or possibly even change Frame to include the previous frame's menu (like how parent is [typically] the previous frame's value).

Also, just to write down a few other thoughts:

Frame.parent could be the actual parent frame rather than the parent frame's value, though care needs to be given to how to handle context-jumping menu items—possibly by:

There could be an explicit Frame._value. If set, Frame.value just returns it (and does not allow assignment?), otherwise it returns Frame.parent[Frame.field] like it currently does. This could actually be the default way to enter a frame (passing a value to the constructor), where the property handlers perform the current behavior iff Frame._value is an instance of a new _IndirectValue (containing both a parent and literal field name). Menu-builders could still keep things simple by doing something like, if a field field is provided, use it for name (if name is not also provided) and construct _value as an _IndirectValue automatically.

The end result could be that every menu entry has a path, used for both msg.add (for callbacks) and for actually matching to what a user types (for literal /-commands and other non-callback navigation).

Optional name. If set, this is used in the user-visible breadcrumbs in msg.path as well as in menu.display.

Optional desc. If set, just used (via msg.add and next to name/path in the parent's list) in Menu.display.

Optional value. If set, used as Frame.value when entered. Can be anything.

Optional parent. If set, used to construct Frame.value as an _IndirectValue, so accessing it (via Frame.value) returns/reassigns parent[Frame.path].

Maybe optional field. If set, used by Frame.value instead of Frame.path?

nmlorg added a commit that referenced this issue Aug 12, 2019
…(field=None) to catch anything not explicitly specified.

Maintain Menu.fields in sorted order, if only to guarantee field=None is the last entry checked. This could have been implemented as explicitly forcing field=None to the end while preserving the rest of the natural order, but I don't think it's really worth allowing non-alphabetical fields in the UI.

See #66.
@nmlorg
Copy link
Owner Author

nmlorg commented Jan 10, 2020

(This is somewhat blocking #83 because I'm using a Menu to choose between entering an event-id-specific or event-title-specific config area. Right now I'm just constructing a dummy Frame, calling Menu.select, then creating new Frames (discarding the one returned by Menu.select) based on the handler.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code changes that improve maintainability without changing behavior
Projects
None yet
Development

No branches or pull requests

1 participant