Skip to content

Conversation

@christianwolters
Copy link
Member

See #883

This unsets the width restrictions from earlier smartmenu works.

With the new css core costummenus, but also dropdowns from smartmenus are rendered without any wrapping.

@christianwolters christianwolters marked this pull request as ready for review April 1, 2025 07:36
Copy link
Member

@abias abias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @christianwolters ,

many thanks for this contribution.

I just rebased the PR onto latest master and decoupled it from #871 by removing the commits which you have submitted in #871 to allow us to concentrate on this single particular bugfix here.

Now, the bug report in #883 touches multiple things which we have to keep in mind when assessing a possible solution:

  • Boost Union must not change Boost core behaviour unexpectedly, that's a consensus. However, Boost Union by design renders the Moodle core custom menu as smart menu (with one navigation level) if a custom menu and a smart menu exists. Boost Union just determines the Moodle core custom menu nodes and the smart menu nodes which should be presented and shows them all together with the smart menu renderers. This was designed intentionally to ease the implementaiton of the smart menus. But against this background, we have to make sure that the custom menu items which are "shipped" as simple smart menu look as if they were custom menus or at least not substantially different.
  • The custom menu in Moodle core does not have a maximum width at all:
    grafik
    From my point of view, this is a downside of the Moodle core custom menu and we do not need to keep this in Boost Union and should however surely have a max width for the custom menu and the smart menu. We should just mention this on https://github.com/moodle-an-hochschulen/moodle-theme_boost_union/blob/main/README.md#exceptions-to-our-main-design-principle
  • In contrast to the pure Moodle core custom menu, the smart menu has a max width. Its value was set in a way that we avoid that the smart menu (which is left-aligned to its main menu item) hangs out of the right-hand edge of the viewport. This basically works well. However, there is #630 as a follow-up issue to give the admin some more control.

Looking at your solution now, I think that neither unsetting the max-width (which would then trigger the problem of letting the menu hang out of the screen) nor setting width to max-content (which would allow the content to explicitely exceed its container width if I am not wrong) is really the way to go.
My gut feeling is that we should rather stick to width: fit-content and raise the min-width of the smart menu for each breakpoint in a way that the menu has more space and looks good even with longer navigation item titles but still does not exceed the available viewport space.

If this is done right, #630 might be obsolete in the end.

@abias abias self-assigned this Apr 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants