am-impact / amnav

Navigation plugin for Craft
168 stars 20 forks source link

Wrong Url in Navigation and no "active" state #19

Closed phaetons closed 9 years ago

phaetons commented 9 years ago

Tried your Plugin and like it alot, but i get two errors in the output: 1 - The URLs in the navigation includes the domain name twice, e.g.:

<a title="Impressum" href="http://www.domain.com/http://www.domain.com/index.php?p=impressum">Impressum</a>

This happens using craft.amNav.getNav() and craft.amNav.getNavRaw()

2 - The class "active" is not set, maybe it's because the url ist not correct.

hubertprein commented 9 years ago

1) Something must be wrong when you insert that node. Did you add it manually? Any specific details you could give on how you added this node to the navigation would be really helpful.

2) This is indeed because of the wrong URL.

phaetons commented 9 years ago

Hmm, i just added the single sections to the navigation via your plugin: craft_amnav

That's my code in the template:

{{ craft.amNav.getNav("footerMenue", {class:'footer-links'}) }}

And thats the output of the standard url output siteUrl: http://test.craft.dev/ entry.url: http://test.craft.dev/index.php?p=impressum

phaetons commented 9 years ago

Can you give me a hint, where (in the plugin services?) the urls are created, so i can check the code?

hubertprein commented 9 years ago

I've been trying to recreate your situation on my end, but had no luck so far. I noticed that your URLs have the page within the URL (index.php in this case), so you're not using the search engine friendly URLs by using rewrite rules from the .htaccess file. Is there a reason you're working like this?

phaetons commented 9 years ago

I use the .htacces that comes with craft, mor_rewrite works, but i had to add 'omitScriptNameInUrls' => true in the config. I cleared the cache but still the same.

So i looked in the database.craft_amnav_pages and field "url" = {siteUrl}http://www.domain.de/index.php?p=impressum

phaetons commented 9 years ago

After adding 'omitScriptNameInUrls' => true in the config and adding a new site to the navigation the field "url" = {siteUrl}http://www.domain.de/team/arnd-rieten

hubertprein commented 9 years ago

Craft caches the part where you change the 'omitScriptNameInUrls' value. Is it possible you can send me a backup of your database, so I can have a quick look to figure out what's going wrong on your end?

phaetons commented 9 years ago

Yes, how can i send you backup?

hubertprein commented 9 years ago

Leave me your e-mail address and I'll contact you.

pixeljitsu commented 9 years ago

Having this same issue. Is there a way to resolve it?

Thanks :)

joshangell commented 9 years ago

+1 - me too

hubertprein commented 9 years ago

Hmm that's weird..

@pixeljitsu , @joshangell Do you have the script name in the URL as @phaetons ? AmNav changes {siteUrl} to your siteUrl in Craft CP or if it's overridden from general.php, it'll use this URL.

In our projects, we change our siteUrl based on the current platform (development, live, etc..). Just by adding the siteUrl to the array in general.php. We add the locale(s) to the array we need and it runs without any problems. Like this: http://cl.ly/image/053a1S3b1X0q Could one of you try this?

phaetons commented 9 years ago

Ok, adding 'siteUrl' to general.php works fine.

'url' in craft_amnav_pages without that:

{siteUrl}http://mydomain.de/team/joshua

'url' in craft_amnav_pages after adding 'siteUrl' and adding a new navigation entry:

{siteUrl}team/susan

Thank you and best regards

hubertprein commented 9 years ago

@phaetons That's good news. I'm glad it worked out.

joshangell commented 9 years ago

@hubertprein Excellent - that worked for me too, thanks! I was actually defining the CRAFT_SITE_URL constant already there, so I just set the siteUrl in the array as well.

pixeljitsu commented 9 years ago

I have it configured like this:

return array(
  '*' => array(
    'cpTrigger' => 'secret',
    'devMode' => false,
    'omitScriptNameInUrls' => true,
  ),
  '.craftdev' => array(
    'devMode' => true,
    'environmentVariables' => array(
      'fileSystemPath' => '/local/public_html/',
      'siteUrl' => "//www.example.craftdev"
    )
  ),
  '.com' => array(
    'cooldownDuration' => 0,
    'environmentVariables' => array(
      'fileSystemPath' => '/remote/public_html/',
      'siteUrl' => "//www.example.com"
     )
  )
);

I'm still having the issue. Not sure what's up.

phaetons commented 9 years ago

siteUrl is an array expecting language codes 'siteUrl' => array( 'en' => 'http://example.com/', 'de' => 'http://example.de/' ),

pixeljitsu commented 9 years ago

Thanks for the help, I'm still having the issue. Here is my general.php:

return array(
  '*' => array(
    'cpTrigger' => 'secret',
    'devMode' => false,
    'omitScriptNameInUrls' => true
  ),
  '.craftdev' => array(
    'devMode' => true,
    'environmentVariables' => array(
      'fileSystemPath' => '/local/public_html/',
      'siteUrl' => array(
        'en' => 'http://www.example.craftdev/'
      )
    )
  ),
  '.com' => array(
    'cooldownDuration' => 0,
    'environmentVariables' => array(
      'fileSystemPath' => '/remote/public_html/',
      'siteUrl' => array(
        'en' => 'http://www.example.com/'
      )
    )
  )
);

Setting the siteUrl to an array as you suggested causes a cascade of errors.

Array to string conversion :
[snip]/craft/app/services/ConfigService.php(681)

Here is the first part of the stack trace:

– [snip]/craft/app/services/ConfigService.php(681): str_replace("{siteUrl}", array("en" => "http://www.example.craftdev/", "{siteUrl}")

676      */
677     public function parseEnvironmentString($str)
678     {
679         foreach ($this->get('environmentVariables') as $key => $value)
680         {
681             $str = str_replace('{'.$key.'}', $value, $str);
682         }
683 
684         return $str;
685     }
686 
hubertprein commented 9 years ago

Move the siteUrl outside the environmentVariables. Place it at the same level as environmentVariables is.

pixeljitsu commented 9 years ago

OK, that gets rid of the errors, thanks!

Just a note, I still have {siteUrl}http://example.com until I delete and then renenter all of the menus manually, but in this case that's not an issue at all.

Thanks again.

hubertprein commented 9 years ago

That's correct.

AmNav stores all URL's in a separate table. When Craft loads a whole bunch of elements with the ElementCriteriaModel, the PHP memory gets overloaded fast if you want more than X elements. By standard, Craft will get 100 elements. If you fetch more, you'll notice the performance will drop drastically. We've had projects that had a huge navigation that went 3 to 4 levels deep, and would fetch a lot of entries. This is why AmNav is not an Element Type and all the URLs are stored in a separate table, because of the performance. Through events (saving / deleting an entry) AmNav will update the URLs in the navigation that should be updated.

Just some information along the sideline, but good to hear your problem has been solved! Closing this issue.