Problem/Motivation

If you enable the 'Hide top bar' option after visiting a node form where the local tasks have been moved to the top bar, the local tasks do not appear as tabs like expected.

Steps to reproduce

On a fresh install:

  1. Login as admin and enable the navigation module
  2. Visit any node. Make note that the local tasks tabs have moved into the Top Bar
  3. Visit the Navigation Settings (Adminstration > Configuration > User interface > Navigation settings)
  4. Enable 'Hide top bar' and Save
  5. Reload the node from step 2

Expected results

Top bar is hidden and the local tasks display as tabs.

Actual results

Top bar is hidden, and so are the tabs. Local tasks are inaccessible.

Proposed resolution

Clearing cache fixes the issue. There is a cacheing issue at play where the top bar, or some sub-element is not declaring the navigation configuration as a cache dependency.

Remaining tasks

Fix it.

User interface changes

When top bar is hidden, local tasks show as tabs. When top bar is present, local tasks show as dropdown.

API changes

None.

Data model changes

None.

Issue fork navigation-3418416

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

m4olivei created an issue. See original summary.

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented
Issue tags: +backend

plopesc made their first commit to this issue’s fork.

plopesc opened merge request !170

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Active » Needs review

Add logic to take into account hide_top_bar property and ensure that local tasks are rebuilt when navigation settings form is submitted.

m4olivei opened merge request !171

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented

@plopsec I took a different approach by instead of invalidating another modules cache tag, properly declaring our cacheability metadata, which I think is the better way to solve this. Let me know what you think. Open to being told I'm wrong 🙂.

https://git.drupalcode.org/project/navigation/-/merge_requests/171

plopesc closed merge request !170

plopesc’s picture

plopesc
he/him
Primary language Spanish
commented
Status: Needs review » Reviewed & tested by the community

@m4olivei Checked your code and it looks cleaner and better for the long run than my original approach.

Closed my MR and marking this issue as RTBC.

Thank you for bringing a second point of view to the issue!

m4olivei’s picture

m4olivei
he/him
Primary language English
Location Grimsby, ON
commented

Thank you for bringing a second point of view to the issue!

My pleasure! Always count on cacheability in Drupal to bring out complications and multiple approaches 😀. Thanks for the review.

  • m4olivei committed c0a84ad9 on 1.x 
    Issue #3418416: Declare navigation.settings as cacheable dependency
ckrina’s picture

ckrina
she/her
Primary language Catalan
Location Barcelona
commented
Status: Reviewed & tested by the community » Fixed

Merged. Thanks!

Status: Fixed » Closed (fixed)

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