Integrate Navigation with Contextual editing

Created on 1 August 2024, 5 months ago

Problem/Motivation

Navigation module does not have a capability to integrate the contextual link Edit.

Steps to reproduce

Proposed resolution

It needs to be added to the Top Bar because it's an action contextual to the content itself.

Remaining tasks

This needs designs. Please don't jump into implementation until there are designs.

User interface changes

Introduced terminology

API changes

Data model changes

Release notes snippet

๐Ÿ“Œ Task
Status

Active

Version

11.0 ๐Ÿ”ฅ

Component
Navigationย  โ†’

Last updated about 19 hours ago

No maintainer
Created by

๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @ckrina
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    I forgot to add the Navigation stable blocker tag.

  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona
  • ๐Ÿ‡ณ๐Ÿ‡ฟNew Zealand quietone

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

  • ๐Ÿ‡ซ๐Ÿ‡ฎFinland lauriii Finland
  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

    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.

  • ๐Ÿ‡ง๐Ÿ‡ทBrazil joaopauloc.dev

    joaopauloc.dev โ†’ made their first commit to this issueโ€™s fork.

  • Pipeline finished with Failed
    about 2 months ago
    Total: 136s
    #324064
  • Pipeline finished with Success
    about 2 months ago
    Total: 693s
    #324072
  • ๐Ÿ‡ง๐Ÿ‡ทBrazil joaopauloc.dev

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States smustgrave

    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

  • Pipeline finished with Success
    about 2 months ago
    Total: 1691s
    #324522
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    Thank for commit.
    I added few notes about js.

  • Pipeline finished with Success
    about 2 months ago
    Total: 717s
    #324680
  • ๐Ÿ‡ง๐Ÿ‡ทBrazil joaopauloc.dev

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

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States bronzehedwick New York

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

  • ๐Ÿ‡ช๐Ÿ‡ธSpain plopesc Valladolid

    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.

  • ๐Ÿ‡ช๐Ÿ‡ธSpain ckrina Barcelona

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

  • First commit to issue fork.
  • Pipeline finished with Failed
    9 days ago
    Total: 223s
    #367213
  • Pipeline finished with Failed
    9 days ago
    Total: 85s
    #367237
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States trackleft2 Tucson, AZ ๐Ÿ‡บ๐Ÿ‡ธ

    Fixed merge conflicts.

  • Pipeline finished with Success
    9 days ago
    Total: 1105s
    #367239
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States trackleft2 Tucson, AZ ๐Ÿ‡บ๐Ÿ‡ธ

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

  • Pipeline finished with Success
    9 days ago
    Total: 653s
    #367281
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States trackleft2 Tucson, AZ ๐Ÿ‡บ๐Ÿ‡ธ

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

  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    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.

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States trackleft2 Tucson, AZ ๐Ÿ‡บ๐Ÿ‡ธ

    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?

  • Pipeline finished with Failed
    8 days ago
    Total: 155s
    #368062
  • Pipeline finished with Failed
    8 days ago
    Total: 146s
    #368073
  • Pipeline finished with Failed
    8 days ago
    #368077
  • Pipeline finished with Failed
    8 days ago
    Total: 151s
    #368078
  • Pipeline finished with Failed
    8 days ago
    Total: 143s
    #368086
  • Pipeline finished with Failed
    8 days ago
    Total: 149s
    #368087
  • Pipeline finished with Failed
    8 days ago
    #368092
  • Pipeline finished with Failed
    8 days ago
    Total: 153s
    #368097
  • ๐Ÿ‡ท๐Ÿ‡ธSerbia finnsky

    I apologize.

    I read the task description poorly.

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

Production build 0.71.5 2024