Integrate Navigation with Contextual editing

I forgot to add the Navigation stable blocker tag.

Changes are made on on 11.x (our main development branch) first, and are then back ported as needed according to our policies.

ckrina → credited lauriii → .

We discussed with @lauriii how to integrate the Contextual editing button into the Top Bar and we came up with this solution that will help unblock the Top Bar and the Navigation from becoming stable: move this Edit button (to show contextual areas) into the more actions dropdown.
The primary action would actually be the button to go to Edit the node if you're on a node, which actually has the same label: "Edit". Which makes it really complicated to understand what is this Edit button.

"Edit" label is problematic

Another problem the UI has is that the same label: "Edit" is used both to show the contextual actions and if you're in a node (the Edit option to go into the node). And it makes it really complicated to understand what is this Edit button and differentiate the 2 of them. So a follow-up would be to change the label for this contextual edit button. If it becomes complicated to find the right copy for this label the discussion should be moved to a follow-up so it doesn't block the Navigation from becoming stable.

Change the controller

Another conversation to have in the future is the UI pattern/controller this should be: it should be a toggle since it's showing and hiding the contextual areas that can be edited (I think for accessibility purposes). But please keep any change to this controller for a followup or we won't get this Stable blocker done.

As a final point, to keep in mind that this (almost not used) feature will progressively disappear with the new functionalities XB will provide.

joaopauloc.dev → made their first commit to this issue’s fork.

Hi folks, follow the image below of the preview editable areas.

Even though this is categorized as a task think there is enough changes here to maybe warrant a simple test case. Least maybe an additional assertion in the module.

Thanks! Looks good I had nothing to add

Thank for commit.
I added few notes about js.

Good catch @finnsky. I forgot the once + context stuff, thanks.
Thanks.

Tested a confirmed the solution works as expected. once() is implemented correctly as well.

Code looks good and it has nice test coverage. Thanks!

However, it only adds the item to node pages.

We might need to discuss whether we need to extend it to other entity types, as the top bar does. Using NavigationRenderer::meetsContentEntityRoutesCondition() would help.

Besides the entity types, do we expect to have it in other pages to allow to modify header or footer blocks? That's the behavior we have now with the toolbar integration.

Adding the "Drupal CMS release target" to see if we could hopefully get to it.

Fixed merge conflicts.

@plopesc I've made the meetsContentEntityRoutesCondition method public, and used it to determine whether to add the links.

Hello! Thank you for work here.

Initially we managed that dropdown via common abstract script:
https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig...

And it worked before. We need to check git history here probably to avoid duplications in common things
This Dropdown should be universal(not related to contextual links) and reusable.

Are you saying that I should move the Javascript into that new file for managing toolbar-dropdown?

No, I mean we can search in the module history.
These links have already worked with the dropdown list. And there is already a script and styles for them.
The backend for this module has changed several times recently, so we should just re-apply it.
I think js is not needed here at all. Maybe only for optimisation.

Are you sure no javascript would be required to find and make visible all of the contextual links on the page when you click the "Preview editable areas" link in the contextual links menu?

I apologize.

I read the task description poorly.

As for MR
JavaScript looks acceptable to me.
+1 RTBC

Checked again the code after talking to @ckrina and we have a few notes about the current MR:

The "Preview Editable Areas" link should appear not only in the entity pages, but in any page, similar to the current "Edit" button added to the to the toolbar. We need to find a way to include it everywhere, disconnected from the entity local tasks.

The "Preview Editable Areas" link text might need to be updated to something like "Hide Editable Areas" while they're visible.

In the current implementation of the "Edit" toolbar button, the state of the contextual editing is stored in the local storage, so if the page is reloaded, o r the user navigates to a different page, the contextual links status persists. Not sure if we actually want to maintain this behavior, but I think it's worth at least to mention it.

Hey @plopesc, thanks for the review.
I'm having a look at the way that the Edit button appears. In the contextual module, they implement the hook toolbar. For the navigation we don't show that toolbar as the other modules do, so the only way that we currently display something similar to the toolbar hook is the top bar item, am I correct? please correct me if I'm wrong.
Since the PageActions plugin displays the Local tasks, how can we implement the edit button without using the local tasks? Should we create another plugin, for example: ToolbarActions, then we could add actions that are similar to the toolbar hook.
Thanks.

Hi, thank you for your interest on this one.

Given that this solution will be probably temporary, we could just provide this feature as part of the existing TopBarItem plugin.
You can just add an extra item to the Local Tasks list calculated using the logic you need, I guess it could be inspired in how contextual's hook_toolbar() works.

If there are no local tasks, you'll have only this new option will be displayed.

Thank you!

Hey @plopesc.

I was investigating how the contextual module adds the Edit button and the contextual link on the pages.
The Edit is added by the hook toolbar(ContextualHooks:19), by default this link is hidden, and then a javascript is responsible for showing or keeping it hidden. If I'm not wrong, the behavior checks if there is any contextual link on the page, and if the Edit is toggle to visible, otherwise keeps hidden. The contextual_preprocess function adds the contextual links if I'm not wrong.
Please, anyone, correct me if my understanding is wrong.

Based on that, for the entities we already have a solution, since the user is accessing an entity page the local tasks will appear and we add the Preview editable areas. Since this is a temporary solution, I don't think it's feasible to implement a "complexity" custom logic just to make this work for other pages. We probably need a javascript code to check if there are contextual links on the page and then control the top bar items. We probably will add a custom logic just for a temporary solution.

So, here is my suggestion. Based on the "complexity" that can be to adding the Preview editable area for non-entities pages, and this is a temporary solution. I would like to suggest that we could move forward with the current solution, displaying the feature only for entity pages.

What do you guys think?

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

OK we discussed in Slack that moving the button outside the dropdown again will make things easier even thoug we moved it in there thinking it'd make it easier.

So the button now should appear on the top Bar, on the left side to be far from the entity actions themselves. Also, instead of having the "Edit" label, it changes to "Editable areas" so it doesn't collide with the entity Edit button. The icon is an eye, which conveys the show&hide action without taking as much horizontal space.

It should change the icon and the aria description or similar based on the state:

I've attached the SVGs too.

Thank you @ckrina.

Moving it outside of the dropdown will make things a bit easier. It could be implemented as a new TopBarItem plugin placed in the Top Bar's Tools region. Also it would allow to reuse, or at least take as inspiration, the JS implementation used for the Toolbar module.

joaopauloc.dev → closed merge request !9992

Thank you for working on this new approach! Good progress here.

Left some comments in the MR.

Hi folks,

I’ve been working on this issue for a while, and it’s almost complete. However, I’m facing a challenge in the final stages and would appreciate any suggestions, ideas, or feedback to help resolve it.

We developed a new Top Bar plugin to display the Preview Editable Areas, which toggles the contextual links on the page if they exist.

The problem:

There is no straightforward way to determine on the backend whether contextual links are present on the page. That’s why we are handling this logic in JavaScript—the same approach used by the Contextual module to decide whether to display the Edit button in the toolbar.

Our current implementation adds the button to the Top Bar as hidden by default. Then, the navigationToggleContextualLinks behavior determines whether the button should be displayed based on the presence of contextual links on the page.

Additionally, there is a CSS rule that changes the display property of the Top Bar to block if the section is not empty. Since :empty considers spaces as valid content, I updated this rule to check for any actual elements instead.

Here’s the updated rule:

.top-bar:has(.top-bar__tools *),
.top-bar:has(.top-bar__context *),
.top-bar:has(.top-bar__actions *)

Everything works fine, but there’s a flickering issue on the screen.

The flickering occurs because the CSS rule initially displays the Top Bar as soon as there is content in the tools section—in this case, the Editable Areas button itself. However, when the JavaScript behavior runs and detects that there are no contextual links on the page, the button is removed. This results in a brief unintended appearance of the Top Bar before it disappears again.

Possible solutions:

I can think of two potential approaches:

1. Prevent the PagePreviewEditableAreas from rendering the button initially. Instead, only add the behavior that checks for contextual links, and then dynamically insert the button if needed. This approach eliminates flickering since the toolbar will only appear after the behavior adds the button.

2. Make the Top Bar “smart” enough to update its display to block only when there is visible content. However, I lack the CSS expertise to determine whether this is possible. Implementing this logic via JavaScript would likely result in the same flickering issue as the current approach.

Any insights or alternative suggestions would be greatly appreciated!

Pushed some css which make work done.

I would like to have some js optimisations here aswell

plopesc → changed the visibility of the branch 3465295-integrate-top-bar-contextual-links-preview to hidden.

plopesc → changed the visibility of the branch 11.x to hidden.

Tested manually and it looks very good after last changes.

We might need some test coverage to consider this one completed, though. This test could be inspired in Drupal\Tests\contextual\FunctionalJavascript\EditModeTest, where the toolbar integration was tested.

Besides that, we need to consider a couple of things:

• Eye icon is not being visible in latest chrome version. See 🐛 The icon for the more actions button is not visible Active
• Icons are being migrated to Icon API 📌 Navigation leverage icon core API Needs work . Might need to take this into account and migrate the eye icons to this new approach if that issue lands first

OK, this is going to come up as a surprise after all the work that has been put in here, but I think this should be closed as won't fix. But let me explain myself before doing a flip table :)
The whole point of this issue was to make sure we don't loose any functionality in core, and that's why we are adding this toggle button into the top bar (as a contextual item). But the real question is: should it be there in the first place? I tried to gather some feedback on the accessibility channel about actually removing it without luck, plus there's 🐛 The toggle that makes Contextual Links visible at all times might not be sufficiently discoverable Active that questions the whole reason of having the toolbar in the first place.

The reasonable direction would be to just finish the issue since you've done a lot of work and it's almost there. But we discussed this with @lauriii and we agreed that this is actually creating a new important problem: it's taking a lot of space on the Top Bar without providing much value, since it isn't a good solution even for the users that would actually need it (per 🐛 The toggle that makes Contextual Links visible at all times might not be sufficiently discoverable Active ).

So better strategy here is to not add this toggle into the Navigation itself, which would effectively remove it from core/Navigation by not integrating it. The we can focus on fixing the problems Contextual Links might have in there. So I'm removing Navigation stable blocker and Drupal CMS release target for now, and I'd move this to Closed (won't fix) as it's creating UX problems.

Finally, and most important, @joaopauloc.dev @plopesc @finnsky and @trackleft2 (and the rest of the people that worked on this): I'm really sorry that this work is not getting in. You've done an amazing work and I really want to acknowledge it. But getting this in this might cause more issues in the future that making the call now and getting rid of it. I hope this doesn't discourage you to keep helping on the Navigation, and if it does feel free to DM me and vent!

Switching to Closed (won't fix).

Sounds good to me, no worries about the sunk cost, thanks for the explanation.