Put browse tab on the menu once and for all

Created on 26 June 2024, 4 months ago

Problem/Motivation

In #3238996: Provide menu link โ†’ we added project browser to the menu. But then in ๐Ÿ“Œ Change "Browse" to be the default action tab for "Extend" RTBC we noticed we weren't doing The Right Thingโ„ข because it was showing up in the new Navigation module when the siblings weren't.

But NOW, if using admin_menu, the tab no longer shows in the dropdown menu for Extend, but its siblings *do*.

Steps to reproduce

Spin up latest 2.0.x and see that it's not on the menu (can also use Try It Now from //dgo.to/project_browser).

Proposed resolution

  1. Identify the correct mechanism to put it on the menu
  2. Implement
  3. Test with navigation module / Drupal 11 AND admin_menu module
  4. write automated test
๐Ÿ“Œ Task
Status

Active

Version

2.0

Component

Code

Created by

๐Ÿ‡บ๐Ÿ‡ธUnited States chrisfromredfin Portland, Maine

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

Comments & Activities

  • Issue created by @chrisfromredfin
  • ๐Ÿ‡บ๐Ÿ‡ธUnited States chrisfromredfin Portland, Maine

    OK so as @lostcarpark points out in Slack, there are three scenarios to test for:

    1. Core Drupal without Navigation module
    2. Enabled (currently Experimental) Navigation module
    3. Enabled Admin Toolbar module

    We are behaving "correctly" for Drupal without Navigation, and WITH Navigation (cases 1 & 2).

    We are a bit different for the "Admin Toolbar" module. In fact, with only Admin Toolbar enabled, there are still no sub-children under the "Extend" menu. The module admin_toolbar_tools is in fact what registers those paths on the menu, by providing its own menu links yaml, which registers some of those core routes: https://git.drupalcode.org/project/admin_toolbar/-/blob/3.x/admin_toolba...

    So, I think there are two possible approaches here - use a menu alter to add our menu link *only if admin_toolbar_tools is enabled* -OR- punt this upstream to admin_toolbar.

    My hunch is that once we're in core, admin_toolbar will probably support adding the menu link to the Browse route in admin_toolbar_extra_tools the same as it does for, say, the current uninstall route.

    One possibility to mitigate why this is an issue for us, is to perhaps enable Navigation in our DrupalPod Try It Now - or NOT enable admin_toolbar_extra?

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

    FWIW I don't think it would make the most sense for a Core module to be altering menu links from a contrib. So I created an issue and a patch for admin_toolbar to try to get it fixed there. There's already a deriver that conditionally adds links for various optional modules, so it fits neatly in there.

    (see https://www.drupal.org/project/admin_toolbar/issues/3457902 โœจ Forward Compatibility with Project Browser Active )

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States chrisfromredfin Portland, Maine

    Thanks, @gslexie! I tend to agree that would be the preferred path forward.

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

    Looks like the update to admin_toolbar_tools has been merged into 3.5, so this issue is probably either fixed or outdated

  • Assigned to chrisfromredfin
  • Status changed to Needs review 7 days ago
  • ๐Ÿ‡ฎ๐Ÿ‡ณIndia narendraR Jaipur, India

    @Chris, can you please close this issue?

  • ๐Ÿ‡บ๐Ÿ‡ธUnited States chrisfromredfin Portland, Maine

    I have noticed it showing properly in admin_toolbar as of late - that's great! I think this is doing what we want now naturally.

Production build 0.71.5 2024