verbb / cp-nav

Control Panel Nav is a Craft CMS plugin to help manage your Control Panel navigation.
MIT License
130 stars 11 forks source link

Icon paths not being correctly handled between environments #119

Closed goraxan closed 1 year ago

goraxan commented 1 year ago

Describe the bug

The plugin is not consistent accross environments, it works "fine" (not 100% fine since the GraphQL icon is broken) in local, but not in other environments. My local environment is Windows, whereas others are Linux. That might be the problem since paths separators change (\ in windows vs / in linux)?

Local display (windows) image

Staging display (Linux) image

The graphql entry has the following project config file

currLabel: GraphQL
customIcon: null
enabled: true
handle: graphql
icon: '@appicons/graphql.svg'
layout: b54f5a17-6a98-4f20-9ca7-195c37575028 # Default
level: 1
newWindow: false
parent: null
prevLabel: GraphQL
prevLevel: 1
prevParent: null
prevUrl: graphql
sortOrder: 18
subnavBehaviour: null
type: craft
url: graphql

The other broken entries have the following project config file

currLabel: Commerce
customIcon: null
enabled: true
handle: commerce
icon: 'C:\path\to\project\vendor\craftcms\commerce\src\icon-mask.svg'
layout: b54f5a17-6a98-4f20-9ca7-195c37575028 # Default
level: 1
newWindow: false
parent: null
prevLabel: Commerce
prevLevel: 1
prevParent: null
prevUrl: commerce
sortOrder: 5
subnavBehaviour: null
type: plugin
url: commerce

And this is how the HTML looks like in other environments tempsnip

Steps to reproduce

  1. Enable the plugin and leave all values by default

Craft CMS version

4.3.6.1

Plugin version

4.0.9

Multi-site?

No

Additional context

No response

goraxan commented 1 year ago

I can confirm this is due to the path separator differences between Windows and Linux. After manually changing the project config files to be "linux-style" everything is working in the other environments (but it's broken in my environment). The GraphQL icon being broken in Windows is due to the same reason (@appicons/graphql.svg is no correctly handled because of the path separator being considered "linux-style").

Since Windows can handle paths both ways (\ or /) I've converted all paths to linux-style. Then I've replaced every DIRECTORY_SEPARATOR occurrency to '/' in the cp-nav/src/models/Navigation.php et voila! fixed :) Appart from that, the project config build has to be changed to build icon paths always in linux-style (I don't know where that happens in the plugin code).

engram-design commented 1 year ago

Interesting! I would've totally thought DIRECTORY_SEPARATOR be environment-aware, which is the reason I've always used it. I'll do some testing on Windows to ensure things are properly handled.

engram-design commented 1 year ago

Fixed for the next release. To get this fix early, change your verbb/cp-nav requirement in composer.json to:

"require": {
  "verbb/cp-nav": "dev-craft-4 as 4.0.9",
  "...": "..."
}

Then run composer update.

goraxan commented 1 year ago

I confirm this is fixed now, thanks!

engram-design commented 1 year ago

Fixed in 4.0.10