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

Add setting to disable hiding untranslated menu items #1063

Merged
merged 5 commits into from
Oct 25, 2022
Merged

Add setting to disable hiding untranslated menu items #1063

merged 5 commits into from
Oct 25, 2022

Conversation

cdauth
Copy link
Contributor

@cdauth cdauth commented Sep 9, 2021

This pull request adds a setting to the admin page where users can configure whether menu items should be hidden when their label (with a fallback to the page title) is not translated into the current language.

The default behaviour in QTranslate was to show such items. The default behaviour in QTranslate-X and QTranslate-XT has been to hide such items. Because of this, the default setting is to hide them.

The issue has been discussed in more detail in #256.

It's my first time contributing to QTranslate-XT, so I'm not sure if I did everything right. In particular, I'm not sure whether/how I should recreate the qtranslate.pot file.

@herrvigg
Copy link
Collaborator

Thank you! The changes look good. I need to read all the story to understand the topic better.

@herrvigg herrvigg added the enhancement New feature or request label Sep 11, 2021
@cdauth
Copy link
Contributor Author

cdauth commented Sep 11, 2021

This is a common use case in smaller non-commercial collectives or associations: The website is written in one main language (for transnational collectives this is usually English), but a small number of important pages (such as the front page) are translated into other languages, so that people who don't speak the main language can still get a rough idea what the website is about. There are not enough resources to translate all content into all languages.
However, many of the visitors are bilingual and speak the main language in addition to their preferred language. Because the website will be shown to them in their preferred language but many pages are not translated into it, those pages should still show up in the menu, so that the visitors can see that this content exists and they can choose to open it if they speak that language as well (or want to run it through a translator).

@herrvigg
Copy link
Collaborator

Thank you for the summary.

As the previous maintainer I also tend to limit the options as much as possible but yes, in that case it makes sense to give this possibility and this seems as the simplest way to do so. If there are many menus and languages we should not impose all the menus to be translated., which might take time and resources. I won't discuss the reasons nor the design for having too many menus, those choices are up to the responsible of the website.

A few questions though.

  1. I also wondered about the option to show the language prefix, You disabled it but it seems we "waste" the option in that case. I don't see any case where you'd want to see this in the menu, it's way too invasive. I'd rather see a flag or a short abbreviation but that's more work. As a baseline I'd rather say the prefix should be implicitly hidden for menus displayed for different languages. The prefix option should only be for posts or other items but not for menus.

  2. There's a piece of code in the menus I need to understand better. We might have to rewrite this differently:

    if ( empty( $item->post_title ) && ! qtranxf_isMultilingual( $item_title ) ) {
    //$item does not have custom menu title, then it fetches information from post_title, but it is already translated with ShowEmpty=false, which gives either valid translation or possibly something like "(English) English Text". We need to translate again and to skip menu item if translation does not exist.

  3. Finally in Menu items without label are removed from the menu #256 there was a mention about taxonomy. Maybe that should be handled similarly, or not.

@cdauth
Copy link
Contributor Author

cdauth commented Sep 15, 2021

  1. Do I understand you right, that you think that the option “Show displayed language prefix when field content is not available for the selected language.” should have no effect for menu items?
  2. I'm not sure how that's related to this issue. How I understand that code is that it is a way to fetch the menu item label in some edge cases. It doesn't seem to me that it has any effect on the issue that this pull request is addressing.
  3. I have no experience with taxonomies in Wordpress and how they are handled by QTranslate. Does anything need to be adapted in this pull request?

@herrvigg
Copy link
Collaborator

  1. Yes that's what I mean. But it turns out to be more complicated than expected. We can't change that without changing the API for several functions. Let's keep it out of scope for now.
  2. Yes this case is tricky and I don't even understand it well... But there's at least something to clarify with the comment in the code which is very confusing.
  3. The taxonomy was mentioned in Menu items without label are removed from the menu #256 so maybe we should combine this to have a unique option. Let's dig a bit more before merge.

@herrvigg
Copy link
Collaborator

I don't forget this, it's almost ready to merge but need a bit more time to check the third point.
I will push a new release without it, I prefer not to rush it to avoid back-and-forth with the user options.

@herrvigg herrvigg force-pushed the master branch 2 times, most recently from 660b9d6 to 2369434 Compare February 19, 2022 23:14
@herrvigg herrvigg added the core Core functionalities, including the admin section label Oct 25, 2022
@herrvigg
Copy link
Collaborator

@cdauth Sorry this has been so long... I have updated the options naming and description but I keep it as a separate option item. About taxonomy it's another topic or a specific use case, so let's merge the new feature. The use case makes sense as your described it, it can be meaningful for many and it's possible to continue building on top of this.

@herrvigg herrvigg merged commit b74ca9f into qtranslate:master Oct 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Core functionalities, including the admin section enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants