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

Do we need to support menu groups? #6

Closed
kirsty-hames opened this issue May 9, 2023 · 2 comments
Closed

Do we need to support menu groups? #6

kirsty-hames opened this issue May 9, 2023 · 2 comments
Labels
question Further information is requested

Comments

@kirsty-hames
Copy link

I can see this got carried over from Boxmenu however I don't think this is needed. We typically use menu groups for two reasons:

  1. Bespoke layouts - if you want to wrap menu items in containers to give more flexibility in the styling of menus (grid layouts).
  2. Section up content.

For both instances, we can do this by using multiple menuBox components. Blocks give us the flexibility with the layout as well as breaking up the menu structure (grouping content).

I've yet to test the group functionality with the Introduce Menus as Pages PR as I wanted to query this before doing so. From a setup perspective, I think I would find it easier to think of menuBox as a component and use blocks for structure rather than enabling the group config I associate with Boxmenu.

Using blocks to group items rather than the group config would give us more flexibility with menu design too e.g. displaying other components between menu items.

I'm not sure what the impacts are with menu locking/completion etc so this will need consideration.

@kirsty-hames kirsty-hames added the question Further information is requested label May 9, 2023
@oliverfoster
Copy link
Member

oliverfoster commented May 9, 2023

Locking: #4

I don't think there would be multiple instances of this particular plugin. This is effectively just the boxMenu behaviour, but as a component, including the grouping behaviour as standard.

Multiple instances are much harder to configure as one would need to refer to sub-pages or sub-menus (groups) by id, or some other filter category. The standard menuBox should be easy to configure in the AAT, by just dropping it onto a page.

More generally speaking, locking is inherited from the course structure and the AdaptModel locking functions along with the other locking attributes, to set _isLocked for each page/menu item, it's much less to do with how the content objects are rendered. i.e. If a content object has _isLocked = true render it as locked.

But yes. Absolutely. I agree with all the aforementioned requirements. If we get the core pr in first, we can then consider how to do a menu with multiple menu components and how to choose the child contentobject for each instance.

@oliverfoster oliverfoster self-assigned this May 9, 2023
@oliverfoster oliverfoster removed the question Further information is requested label Jun 19, 2023
@oliverfoster oliverfoster removed their assignment Jun 19, 2023
@oliverfoster oliverfoster added the question Further information is requested label Jun 19, 2023
@kirsty-hames
Copy link
Author

Closing as we'll need to maintain groups if we can't have multiple component instances.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants