Problem/Motivation

With https://www.drupal.org/project/drupal/issues/3028191 a new UX feature has been introduced to reduce contextual-links noise on layout_builder pages. The chosen selector to remove the contextual links 'layout_builder_block:' does also remove the links provided for translation. This makes this module unusable, thats why I tag this issue as a Bug report.

Steps to reproduce

- Vanilla Drupal 10.2
- Install this module, minimum 2 languages and configure content translation
- Create a node with an overriden layout, a translation and try to translate the inline blocks

There should be no contextual translation links anymore.

Proposed resolution

I have no Idea. I guess it's not really possible to fix this 'clean' in this module. I guess the core patch for st translations should change the change and limit the selector to 'layout_builder_' or something like that.

Temporary solution

For now I can share my ugly, temporary solution which I know is not good practice but makes it work for the moment. Just add the following JavaScript to any js file loaded in layout_builder. For instance in your frontend theme.

  $(window).off('drupalContextualLinkAdded');

  // Remove all contextual links outside the layout.
  $(document).on('drupalContextualLinkAdded', (event, data) => {
    const element = data.$el;
    const contextualId = element.attr('data-contextual-id');
    if (contextualId && !contextualId.startsWith('layout_builder_')) {
      element.remove();
    }
  });

This code does what cores tries to do, but with a suitable selector. We prevent core from doing it's job by removing the event from window since it's triggered also for document we can do this. As far as I can tell, no other core function relies on this, but I can't tell which side effect this might have on contrib modules.

CommentFileSizeAuthor
#31 claro-removing-contextual-links.patch1.48 KBdan612
#26 3411037-contextual-links-removed-from-core-26.patch1.97 KBsmulvih2
#14 3411037-contextual-links-removed-from-core.patch1.54 KBcarolpettirossi

Issue fork layout_builder_st-3411037

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

Hydra created an issue. See original summary.

Hydra opened merge request !5

hydra’s picture

hydra commented

I decided to add this as a MR for this module, so people can get a patch for the workaround and don't need to add the code by them self.

tomsaw’s picture

tomsaw commented

Same here. Thanks for the workaround @Hydra! Saved my evening.

millerrs’s picture

millerrs commented
Priority: Normal » Major

Saved me as well. Content editors reported this issue after updating to Drupal 10.2. A big thank you to @Hydra. Also, let's escalate the severity of this issue. Without this patch, users cannot translate layouts, which is a key functionality of this module.

joe_carvajal’s picture

joe_carvajal
He/him
Primary language Spanish
Location Seville (Spain)
commented

This one did the trick for me too, thank you!

millerrs’s picture

millerrs commented
Status: Active » Reviewed & tested by the community
kmonty’s picture

kmonty
He/Him
Location Berlin, DE + SF, CA
commented
Status: Reviewed & tested by the community » Needs work

I'd say this is `Needs Work`, as this is an `ugly, temporary solution`.

At the very least, it should remove the dependency on jQuery.

owilliwo’s picture

owilliwo
Location Montreal
commented

Patch is working well on my english/french site !
Thanks a lot.

Here is my "Layout Builder Stack":

Layout Builder (layout_builder)                                                      Enabled    10.2.2          
Layout Builder Restrictions (layout_builder_restrictions)                            Enabled    8.x-2.19        
Layout Builder Restrictions By Region (layout_builder_restrictions_by_region)        Disabled   8.x-2.19        
Layout Builder Symmetric Translations (layout_builder_st)                            Enabled    8.x-1.0-alpha3  
Layout Builder Styles (layout_builder_styles)                                        Disabled   2.0.0
junkuncz’s picture

junkuncz
Primary language Hungarian
Location Budapest
commented

Works for me as well, thanks!
I agree with @kmonty regarding the refactoring so jQuery should be dropped out (as far as I know vanilla js should be enough there).
End of security support for 10.1.x. is closer than you think so I suggest to remove jQuery then create a follow-up ticket to do a proper fix and merge this MR (of course recheck it first).

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Needs work » Reviewed & tested by the community

I agree, the dependency on jquery should not block moving on here. Setting back to RTBC then.

slideaway’s picture

slideaway commented

@hydra Could you clarify what I need to do to install this fix? I've never installed a fix like this and haven't had any luck trying to implement it.

hydra’s picture

hydra commented

@slideaway Oh you need to download the patch from the merge request. Here is some documentation on how to do this: https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... And of course you need to apply the patch to your version of layout_builder_st.

carolpettirossi’s picture

carolpettirossi commented
StatusFileSize
new 3411037-contextual-links-removed-from-core.patch1.54 KB

Downloading the diff and uploading the patch to your local repo is the recommended approach.
However, I'm attaching the patch from the MR here in case someone is used to this old approach of adding patches to the issue.

The code works successfully in my case. Can't wait to see this merged and released.

slideaway’s picture

slideaway commented

I was able to successfully add the patch and got things working again, thanks for aiming me in the right direction @hydra.

And thank you @carolpettirossi, I didn't actually know that was the old approach versus the new preferred way. I'd just been adding them to my composer.json using composer-patches from support threads like this, so I was unfamiliar with how to do it the way Hydra had mentioned.

nelo_drup’s picture

nelo_drup commented

In my case it doesn't work at all, the tranleta option still doesn't appear or maybe I need to configure something else

igorgoncalves’s picture

igorgoncalves commented

@carolpettirossi patch fixed the contextual link issue and they're back!
Thanks carol.

But in my project it ending up showing another issue after trying to editing blocks using those links, and this issue/patch helps me:

https://www.drupal.org/project/drupal/issues/3208687

sharing just in case another one faces the same.

heddn’s picture

heddn
Primary language English
Location Illinois
commented

+1 on RTBC. LGTM

juagarc4’s picture

juagarc4 commented

Hi all,

The patch #14 works in my project properly and no other issues were found by testing it.

Context:
Drupal 10.3.5
Layout Builder ST: 1.0-alpha3
PHP 8.3

+1 on RTBC.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Reviewed & tested by the community » Postponed
Related issues: +#3431600: Automated Drupal 11 compatibility fixes for layout_builder_st

I've included this fix in the Drupal 11 compatibility merge request.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Version: 8.x-1.x-dev » 2.0.x-dev
Status: Postponed » Needs review

Ok, so now that we have a 2.0.x branch , (working), upgrade to 2.0.x, run drush updb first before a cache rebuild, then rebuild cache, review the MR 5

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs review » Reviewed & tested by the community

Restoring back to RTBC, since this was RTBC previously.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Something may have happened after D10.2 because even with this patch installed, I do not see the contextual link for translation. Probably due to an upstream change they probably finished it off.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Priority: Major » Critical
Status: Reviewed & tested by the community » Needs work

This workaround fix appears to have been broken some time after Drupal 10.2

I no longer see the translation contextual link with or without the patch using any version of layout_builder_st and Drupal 10.4.x / 11.1.x .

liam morland made their first commit to this issue’s fork.

smulvih2’s picture

smulvih2
Location Canada 🍁
commented
StatusFileSize
new 3411037-contextual-links-removed-from-core-26.patch1.97 KB

Adding a patch here for alpha3 that excludes the last commit that adds to .gitlab-ci.yml, since this file doesn't exist in the alpha3 branch. This is needed for WxT 5.4.x.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

@smulvih2 , not sure if this patch even works anymore, although it doesn't appear to cause any harm.

joel_osc’s picture

joel_osc commented

FYI I think this just broke 5.3 wxt builds because they use:

            "drupal/layout_builder_st": {
                "3420063 - Call to a member function getConfig() OverridesSectionStorage.php":
                "https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/6.diff",
                "3411037 - Fix core removing contextual translation links":
                "https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/5.patch"
            },

I have had to add it to the patches-ignore section of the local composer.json for now to get builds to work.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

@joel_osc , actually I think it's MR6 that is the gotcha , @liam morland combined two patches into one. With that said, I'm going dizzy looking at patches all day long.

Ideally some of these fixes eventually get merged so that we don't have to use so many patches.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I haven't worked on merge request 6.

Composer files shouldn't refer to patches like: https://git.drupalcode.org/project/layout_builder_st/-/merge_requests/5....

These change anytime someone makes a commit so you can't be sure you'll get the same code each time you build.

dan612’s picture

dan612 commented
StatusFileSize
new claro-removing-contextual-links.patch1.48 KB

I think the issue is in claro_preprocess_block().

This patch (which is taken from the patch here) brings them back. They seem to be inconsistent - sometimes have to refresh the page a few times before they show up...but from what I can tell they always come back eventually. Patch is for 10.4.x.

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

The merge request currently targets 8.x-1.x. It should probably target 2.0.x.

  • heddn committed fcc68b90 on 8.x-1.x authored by hydra 
    Issue #3411037 by hydra, heddn: Contextual links for translation are...
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs work » Fixed

This resolves some of the issues. Working to clean-up as many errors as possible so the D11 upgrade path doesn't have problems.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

@heddn, why was this merged into 8.x-1.x ?

2.x-dev has had a lot of work gone into it.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Fixed » Needs review

  • heddn committed b4cecbea on 2.0.x 
    Issue #3411037 by hydra, heddn: Contextual links for translation are...
heddn’s picture

heddn
Primary language English
Location Illinois
commented
Status: Needs review » Fixed

Merged into 2.x as well.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Thanks

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.