Discussion about merging the module with Menu block current language

This discussion is referring to https://www.drupal.org/node/2814153 issue.

We could of course discuss to merge the two modules, taking the best from both

I think the biggest issue with merging these modules is that they inherently do different things (different approach) for the same issue.

So if we want to merge these modules, we should decide which approach would be better/more dynamic, but I'm not sure if it's necessarily a bad thing to have competition (especially when our modules do very different things).

Here is some thoughts on points you raised in #2814153: New advanced/multilingual option: Only display translated menu items:

I'm gonna refer Menu block current language as MBCL.

For instance support for fallback languages, override settings for single menu items, and there are also some cases that needs special treatment, like links to translatable forms (where we need to check if the form is translated, and not the content).

I agree, but it would be almost impossible to support every corner case and in my opinion it's fine if end-user has to handle the special cases by themself.

MBLC provides an event subscriber event that lets users choose whether the menu link should be visible or not (with something like this):

my_module.service.yml:

services:
  my_module.mbcl_visibility_subscriber:
    class: '\Drupal\my_module\EventSubscriber\VisibilitySubscriber'
    tags:
      - { name: 'event_subscriber' }

my_module/src/EventSubscriber/VisibilitySubscriber.php:

namespace Drupal\my_module\EventSubscriber;

use Symfony\Component\EventDispatcher\EventSubscriberInterface;
use Drupal\menu_block_current_language\Event\HasTranslationEvent;
use Drupal\menu_block_current_language\Event\Events;

/**
 * Event Subscriber VisibilitySubscriber.
 */
class VisibilitySubscriber implements EventSubscriberInterface {

  /**
   * Dispatch Events::HAS_TRANSLATION event.
   *
   * @param \Drupal\menu_block_current_language\Event\HasTranslationEvent
   *   The event to dispatch.
   */
  public function onRespond(HasTranslationEvent $event) {
    $link = $event->getLink();

    // Case where menu link should not be visible.
    if (my_special_case) {
      $event->setHasTranslation(FALSE);
    }
    // Where already hidden link should be set visible.
    if (me_second_special_case) {
      $event->setHasTranslation(TRUE);
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events[Events::HAS_TRANSLATION][] = ['onRespond'];
    return $events;
  }

}

We could try merge functions into menu block module, maybe as a submodule, but in that case I hope the maintainer is prepared to support a range of different multilingual issues, several issues also beyond the menu block itself. For myself I think multilingual menu issues needs a dedicated project dedicated to solve the a range of issues.

The best place to fix this would be something like i18n (i18n_menu) was for Drupal 7, so I agree Menu block might not be the ideal place to fix this.

At the moment MBCL has to be standalone module, because neither the core or the Menu block provides any way to alter the menu tree manipulators (would be kinda easy to fix tho).

Also, MBCL supports couple of features that are not yet implemented in core (#2594425: Add the option in system menu block to "Expand all items in this tree" and #2631468: Menu subtrees in menu blocks show all subitems regardless of the active menu item), but most likely will eventually end up in core.

For the sandbox module I posted it doesn't add a replacement for core's menu block, it adds a new configuration option inside the core's menu blocks. So the functions can be combined with, for instance, this `Menu Block` module.

This is definitely the most compelling argument for your module.

It looks great with support for string translations for the core menu links that have hardcoded labels.

It should support any menu link that extends the Drupal\Core\Menu\MenuLinkDefault class (core, contrib or custom).

I'm not really sure what the option "views" is and how it should be used in menu blocks?

It's a menu link type (Drupal\views\Plugin\Menu\ViewsMenuLink) that you can create with the Views module (View -> Page -> Menu -> Normal menu entry).

I also think the module miss a distinction between controlling visibility depending on weather the content or the label has been translated.

That's by design. MBCL does not care if the actual content behind the link has translation or not, just that the menu link itself has a translation.

It would be very hard to distinguish whether the content is translated or not (thus making it very unrealiable), because the menu link could link virtually to anything (custom entity, page callback) and the menu link is not guaranteed to be something that could even be access checked, like absolute url.

I think this is something that should be handled with permissions and there are a few modules that do it already: https://www.drupal.org/project/content_language_access for example.