-
-
Notifications
You must be signed in to change notification settings - Fork 138
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
Feature suggestion: Make wagtailmenus more useful in multilingual projects where multiple languages share the same Site #242
Comments
I think the aim is 'out-of-the-box' compatibility with something like Perhaps the best approach here will be to create an optional sub-app within wagtailmenus (e.g. |
Now that #258 is resolved, I'll try to make some progress with this soon :) |
hey @ababic - i would be interested in this. One possible solution could be to have multiple main menus. And add a handle field for each main menu created. Then it would be up to the template to decide which one to use, like so: Flat menu seems to have similar functionality - can you maybe just add that to Main menu? |
actually.. if the Flat Menu has all the fields, then i could just do it using Flat menu.. hmm. I will give it a try in a few days. |
@robnardo I think using flat menus is a common way around this problem, although it's not great for developers to have to assemble the correct handles every time. It would be much nicer to use the active language to get the correct version automatically. I've closed this just because I haven't had the free time to spend on feature development for a while, but I would be happy to open this issue again if you think it would be useful. I suppose someone may pick it up an submit a PR one day 😃 |
Someting like this would be useful. In my solutions I've used |
I think wagtail is close to figuring out what it wants to do natively for translations/localisation, so it's probably worth holding out for that. |
How is it suggested we deal with menus when we have regional websites at different subpaths. We have our North American English at the path How can we construct multiple "main menus" for each regional website. Should we not use main menus at all and construct flat menus for each regional main menu? Any thoughts would be greatly appreciated. |
Hey @calbear47. That isn't really something wagtailmenus caters for out-of-the-box. I think main menus could work, but you'd need a custom menu model (with relationships/unique criteria overridden, and certain methods overridden to take into account the 'current region' from the request), plus custom admin views for switching between different site/region combinations in the CMS. |
@calbear47 Whatever native solution we come up with for |
@ababic Ok, thank you for your input and direction. |
I'm using the official wagtail approach to i18n, and for the menus to work I followed these steps: And for the new classes I put: # I don't like that in the example they import this as "settings"
from wagtailmenus.conf import settings as wagtail_settings
class LocalizedMainMenu(AbstractMainMenu):
pass
class LocalizedMainMenuItem(AbstractMainMenuItem):
menu = ParentalKey(
LocalizedMainMenu,
on_delete=models.CASCADE,
related_name=wagtail_settings.MAIN_MENU_ITEMS_RELATED_NAME,
)
def __init__(self, *args, **kwargs):
"""
This makes the menu work seamlessly with multi-language entirely:
- In the admin you only add the pages once per language, but the
menu also appears even if you are visiting the page that is in
another language.
- The menu item's text is in the current language.
- The menu item's link point to the right language.
"""
super().__init__(*args, **kwargs)
if self.link_page and self.link_page.localized:
self.link_page = self.link_page.localized Then, just create the main menu as you would normally, that is: each menu item just once for each page, and select the page in one of the languages (any should work, but I always select the main one). Until wagtailmenus officially supports wagtail's i18n, this might be a temporary solution. |
Thanks for sharing @perepicornell! That's superb :) |
My only concern there would what happens when you edit and resave the menu. Don't the stored pk values change depending on what language is active? Does that have any negative impact? |
Overriding |
That's a very good point, I didn't verify it but saving a menu in admin will override the pages to the one of the language you are using. And yes, I was taking a look at |
Hi! I'm sorry I don't fully understand the issue, but it's working like a charm for me. If it helps, I'm only using the locale codes so far, so the language switching menu says "CA - ES - EN". |
Hi again! The trick of extending AbstactMainMenuItem to change its self.link_page to self.link_page.localized introduces a bug that prevents menu items with subpages to be displayed. Say you save a menu while using the admin in english language, now in the Menu model you have the english versions of the Page objects. Then you visit the site in catalan, now the page that was ID 30 in english is ID 8 in catalan.
That's when it checks this: try:
item.link_page = self.pages_for_display[item.link_page_id] Which, in the case described, fails and the menu item is not displayed. I fixed in by overriding if item.link_page.localized:
item.link_page = item.link_page.localized And the final result is this: class LocalizedMainMenu(AbstractMainMenu):
def get_pages_for_display(self):
"""Returns a queryset of all pages needed to render the menu."""
if hasattr(self, '_raw_menu_items'):
# get_top_level_items() may have set this
menu_items = self._raw_menu_items
else:
menu_items = self.get_base_menuitem_queryset()
# Start with an empty queryset, and expand as needed
queryset = Page.objects.none()
for item in (item for item in menu_items if item.link_page):
if item.link_page.localized:
item.link_page = item.link_page.localized
if(
item.allow_subnav and
item.link_page.depth >= wagtail_settings.SECTION_ROOT_DEPTH
):
# Add this branch to the overall `queryset`
queryset = queryset | Page.objects.filter(
path__startswith=item.link_page.path,
depth__lt=item.link_page.depth + self.max_levels,
)
else:
# Add this page only to the overall `queryset`
queryset = queryset | Page.objects.filter(id=item.link_page_id)
# Filter out pages unsutable display
queryset = self.get_base_page_queryset() & queryset
# Always return 'specific' page instances
return queryset.specific()
class LocalizedMainMenuItem(AbstractMainMenuItem):
menu = ParentalKey(
LocalizedMainMenu,
on_delete=models.CASCADE,
related_name=wagtail_settings.MAIN_MENU_ITEMS_RELATED_NAME,
)
def __init__(self, *args, **kwargs):
"""
This makes the menu work seamlessly with multi-language entirely:
- In the admin you only add the pages once per language, but the
menu also appears even if you are visiting the page that is in
another language.
- The menu item's text is in the current language.
- The menu item's link point to the right language.
"""
super().__init__(*args, **kwargs)
if self.link_page and self.link_page.localized:
self.link_page = self.link_page.localized I know it's not the right way (I would normally never override a method by copying its entire code), but it was already very hard for me to understand all of that and find a workaround because I'm not that good at it yet. Now it works fine, but if anyone can help me in finding the right way for solving this I will really appreciate it! Many thanks 😄 |
Hey @perepicornell. I'm sure this will be really helpful for others... thanks for doing such a great job of explaining the problem and your solution. |
Good day @perepicornell! Thats what i meant:). Thank you, works great, especially after commenting string super().get_top_level_items(), because it causes recursional reqests therefore my DB goes down: Mar 10 15:04:08 ant gunicorn[245124]: File "/var/www/example.com/portfolio/home/models.py", line 297 in get_pages_for_display |
Thanks @noparamos ! Damn, I put this line just to jump to the reference easily in PyCharm and I forgot to remove it before copying 😅 |
Hi there, I use Wagtail-Localize and Wagtailmenus together. I found a solution to make the menu localized but it's so easy that I'm pretty sure I have forgotten something 😅. I have created a Main Menu and the following template:
Using link_page.localized, the url and title that are displayed depend on the location code. I only create one menu and let the translation happen in the template. I don't beleive this workarround to be so easy. Where did I miss something? 😂 |
@Redjam I would have a scan at the code suggestions further up, specifically this one. Wagtailmenus does some pre-fetching of page data to use for rendering, so ideally you need to swap the pages out before it gets to the template, so that the correct pages are prefetched in the first place. Your solution will work for a simple menu, but not for one with multiple levels, because the localized page descendants will not have been prefetched. |
@ababic many thanks for your feedback. I'll dig into the subject to improve my code. |
@perepicornell trick didn't work for me. I struggled to debug but I found what was causing the issue. In the line I can't explain why I had to make this change 🤷🏻♂️ Maybe it will help someone else. Pyhton version used: Python 3.8.12 |
I'm using The problem is in If I understand correctly, Until there is better solution, a quick & dirty workaround could be replacing the line with something like: base_page_queryset = self.get_base_page_queryset()
suitable_pages_ids = [p.localized.id for p in base_page_queryset]
queryset = queryset.filter(id__in=suitable_pages_ids) |
With replace Here is a working solution for me on main menu, I don't know it is OK or not.
{% load menu_tags %}
{% for item in menu_items %}
<a class="navbar-item {{ item.active_class }}" href="{{ item.href }}">{{ item.text }}</a>
{% endfor %}
# wagtailsmenus/models/menus.py
# ---------------------------------------------------------------------
# Set 'text' attribute
# ---------------------------------------------------------------------
if item_is_menu_item_object:
item.text = item.menu_text
else:
# i18n update, since page always return title as the default value.
if page.localized:
item.text = str(page)
else:
item.text = getattr(item, settings.PAGE_FIELD_FOR_MENU_ITEM_TEXT, item.title)
# ---------------------------------------------------------------------
# Set 'href' attribute
# ---------------------------------------------------------------------
if option_vals.use_absolute_page_urls:
item.href = item.get_full_url(request=request)
else:
# i18n update, also we can get the url of the localized page.
if page.localized:
item.href = page.localized.url
else:
item.href = item.relative_url(current_site, request=request) |
Hi there. Should there be a pull request with this? This would be rather useful in the mainline. @hoiwanchang ? @ababic, since this issue has been open for about six years, are you aware of an effort to make it work in the mainline? Thanks, |
Hi @goutnet. Not that I know of. I could never really settle on an approach for multilingual projects, and eventually abandoned the idea.. |
I'm considering adding a
language
field to both AbstractMainMenu and AbstractFlatMenu, which would default to the default language for the project, then altering the unique constraints on each model to allow menus to be created for unique combinations ofsite
andlanguage
(orsite
,language
andhandle
in the case of AbstractFlatMenu).The
{% main_menu %}
and{% flat_menu %}
tags andget_for_site()
methods on the related models should then be updated to identify the current language and use it to identify the relevant menu object.This behaviour would perhaps be 'toggleable' via a new setting (disabled by default) so that users wouldn't have to be concerned.
This same setting could also be used to conditionally customise the CMS UI, revealing the language field for flat menus in the add/edit UI, and also adding a language filter to the filters list.
The UI for main menus presents a more complicated problem. Right now, using the 'site switcher' select in the corner always creates a menu for the selected site if it didn't already exist, which I'm not sure is the best way to cater for multiple languages - I think the UI might need need to indicate more to the user than a single select can provide concisely, like which sites menus for which languages already, and which don't. Again, this will have to be explored.
The text was updated successfully, but these errors were encountered: