Integrate Gin with the new Navitaion Top Bar

I'm working on it.

So far I've pushed the basic support for the new regions in the top bar and to disable the toolbar when navigation top bar is active.

Hey @Jürgen 👋

Can you take a look here?

You'll need to install navigation and navigation_top_bar

@saschaeggi yes, this is on my list for this week.

This is looking great, I like this a lot.

However, I found a couple of issues and my test environment looks like this:

• Drupal 11.x-dev
• Gin with the MR!589
• Enabled navigation and navigation_top_bar

The issues I found:

• When loading a page, the main region takes the full browser width, until the left navigation gets loaded, which is when the main region flips back to the narrower width. I guess that's an issue with navigation, not with Gin, right?
• The action buttons are clickable but nothing happens.
• When opening/closing the right sidebar, this works correct both throws this exception in the browser console: Uncaught TypeError: trigger is null more_actions.js:50:7

Looking into the MR, I wonder how resistant this code is for the various combinations that we may have to support:

• D10 with Gin only
• D11 with Gin only
• D11 with Gin and navigation
• D11 with Gin and navigation and navigation_top_bar

Is there any other combination that I may have missed?

@saschaeggi found the reason for the nonfunctioning action buttons. They no longer have the data-drupal-selector="gin-sticky-${formElement}", but instead data-drupal-selector="${formElement}", i.e. the gin-sticky- prefix is no longer there. I guess, similar changes elsewhere will have additional side effects that we don't want.

Before we can fix those, we should finalize the scope definition on what combination we want to support as raised in #8. We can then implement that accordingly, but I can already foresee that we have to cover a lot of different scenarios with one code base.

That said, I wonder if we should move the navigation support into Gin 5.x and there only support 11.2 and later. Or whatever Drupal core version will have the new navigation in a stable fashion.

This is way too much CSS/twig for me to competently review, but I came here to say that the main pain I see without this fix is the page title not being displayed by the top bar on admin pages.

I've noticed a small issue using drupal 11.1.7, Gin 4.0.x-dev and the patch from MR589. There is no right-padding in the top bar which causes the local actions being pushed to the edge of the browser.

It seems this can be solved with a max-width: 100% on the top-bar class.

Just was testing Drupal CMS on the 2.x branch, which is on 11.2.0-beta, so it has nav top bar enabled by default. There are quite a few issues to resolve and I'm not entirely sure what the correct state is for testing.

Should Gin Toolbar be installed? If so then Toolbar also must be installed which seems like a problem. But without Gin Toolbar, the navigation doesn't get Gin's styles outside of admin pages.

I tried with this patch to see if it resolved things but it doesn't seem to make much difference. What I'm seeing is basically an empty top bar on all admin pages. I'm not seeing the action links appearing in the toolbar as in the other screenshots posted here.

This is with Gin Toolbar and Toolbar uninstalled.

Tested what happens when you update an existing site to 11.2 and updating this to a bug based on that. I think it's major since it's a pretty big regression.

On admin pages, it is the empty toolbar as shown in the previous comment.

When viewing site pages, you get the double toolbar:

Escalating even further. I guess this can't block a Drupal CMS release, since existing sites updating to Drupal 11.2 will encounter this, but hopefully we can get it solved sooner rather than later.

We've discussed this today and our plan is this:

• We create a Gin 5.x branch
• We get this branch ready for Drupal 11.2 and later
• This will work with our without the navigation being enabled
• It will not work with Drupal before 11.2 any more
• Likewise, we publish Gin 4.1 which only works with Drupal before 11.2

I'll be unavailable for the rest of the week, but will get into this first thing next week, and we will do our best to get this done asap.

It seems like the decision may affect Gin toolbar module, which is used on 60k+ sites and offers horizontal and vertical menu. If removed, it will cause upgrade issues for sites which are currently using it. Is there any plan to keep/or add back the possibility to have also the horizontally oriented menu in such case? Navigation itself currently does not support horizontal menu, only vertical, which may be quite restrictive, given the original core's Toolbar module offers both options (see the comment #88 📌 New Toolbar Roadmap: Path to Beta & Stable Active ).

@poker10

Navigation itself currently does not support horizontal menu, only vertical, which may be quite restrictive, given the original core's Toolbar module offers both options

Navigation will soon be the default in core and toolbar will get deprecated and eventually removed. So the path forward is the vertical navigation

t seems like the decision may affect Gin toolbar module, which is used on 60k+ sites and offers horizontal and vertical menu. If removed, it will cause upgrade issues for sites which are currently using it. Is there any plan to keep/or add back the possibility to have also the horizontally oriented menu in such case?

Until toolbar gets eventually removed from core we'll at least support the regular toolbar options but will remove our additional own custom implementations around toolbar (vertical, our own test navigation implementation etc. to reduce variants and make it easier to maintain)

This MR!589 can now be tested together with MR!60 for the Gin Toolbar from 📌 Disable in Drupal 11.2 when navigation is enabled Active .

I've tested this with vanilla Drupal 11.2-RC2 and with Drupal CMS 1.2-RC, and all seems to be working fine in both cases, navigation module being enabled or disabled.

How to test? Checkout the MRs for Gin and Gin Toolbar and then give it a try.

My suggestion is to merge both MRs asap, as this will make testing much easier as people could then just use the dev releases which wouldn't break anything else, as they come in new major branches. After that, we could address remaining issues in separate issues and make life for everyone much easier. Waiting for at least one RTBC in both issues, though.

Tested this in Drupal CMS. It is already much better and I agree re: merging to make it easier to test because it was a pain to get it going to test myself :)

The main issue is that without Gin Toolbar, the navigation loses the Gin styling on front-end pages, so it gets the Claro styling instead.

The main issue is that without Gin Toolbar, the navigation loses the Gin styling on front-end pages, so it gets the Claro styling instead.

That's a limitation of Drupal we can't overcome in contrib

Right but we still have the module, can't we use it to add the styles but nothing else?

@pameeela I had been too aggressive in disabling components of the gin_toolbar in my previous attempt. I've fixed that now. Please pull the latest MR from the gin_toolbar module, and styling should be fine, also in the frontend.

However, there are still some javascript issues that I'm working on now.

OK, javascript errors and sticky action buttons are now fixed. Over to you @pameeela for another round of testing, please. After that, we probably gonna merge this and tag alpha releases so that we get a wider testing audience.

Could you tag stable, not alpha? An alpha won’t allow users of Drupal CMS with existing projects to update. :-/

I know, @phenaproxima, but stable would be premature as of today. But if we get some feedback that the current state is at least working as good as version 4, then we will tag 5.0.0 right away. Even as soon as later today, so that we're ready for Drupal 11.2

But we need some confirmation that it is working alright, otherwise a stable release would create the best impression.

Thanks @jurgenhaas, I have tested and this is looking great! I think an alpha is a good next step.

Thank you @pameeela for testing. I've merged this and will tag an alpha right now.