verbb / navigation

A Craft CMS plugin to create navigation menus for your site.
Other
90 stars 22 forks source link

Query returns NULL in CP preview #384

Open klick opened 7 months ago

klick commented 7 months ago

Describe the bug

In my templates I am determining the current node id this way:

{% set currentNavNodeId = craft.navigation.getActiveNode({ handle: 'mainMenu' }).id %}

which works fine so far. The template breaks though in the CP's preview panel as well as the parameterised preview where craft adds info to the url (aka ?x-craft-preview=E8eOWX…) after applying any change to the entries content.

Steps to reproduce

  1. Editing Entry in the CP
  2. Click in-built “Preview” or “View → Primary entry page”
  3. Twig Runtime Error : Impossible to access an attribute ("id") on a null variable.

Craft CMS version

4.7.1

Plugin version

2.0.25

Multi-site?

Yes

Additional context

The navigation elements are entries.

engram-design commented 7 months ago

That would be correct behaviour if there is no active node. You probably shouldn't assume that craft.navigation.getActiveNode({ handle: 'mainMenu' }) always returns something, hence the error message that it's trying to get id from null. The two possible values from getActiveNode() are null or a NodeElement object.

I'd adjust your templates to handle this with a conditional, or a null coalescing operator.

{% set currentNavNodeId = craft.navigation.getActiveNode({ handle: 'mainMenu' }).id ?? null %}
klick commented 7 months ago

Thank you for replying.

I could do that but would not be able to correctly render a preview. Why is there suddenly no NodeElement when in preview mode i wonder.

engram-design commented 7 months ago

I do mean that you should be doing that anyway, as you might be on a page that doesn't have an active node, and you'll run into this error sooner or later.

As for why it doesn't work in preview, I'd say it's likely because we don't handle it, and we expect it to be a front-end request. I'll have a look into adding support for that.

klick commented 7 months ago

For the time being I have now resorted to hard code the ID into the templates → {% set currentNavNodeId = 10987 %}. Wich is not preferred as you can imagine but works as expected. I’ll watch this place for changes.

Thanks.