dniccum / nova-documentation

A Laravel Nova tool that allows you to add markdown-based documentation to your administrator's dashboard.
MIT License
37 stars 15 forks source link

Package doesn't correctly handle custom nova.path #26

Closed dm-pf closed 1 year ago

dm-pf commented 1 year ago

Hi,

I believe this might be a regression caused by PR #25 , but cannot be sure because it's our first time using this package.

We have a custom nova.path as follows: config/nova.php : path => '/' so one would expect the package to be available at example.com/documentation but it is rather available at example.com/nova/documentation.

Bad sidenav link

The sidenav link is correctly set as /documentation but when clicked it hits a 404 error.

artisan route:list shows:

Screenshot 2023-02-23 at 15 03 33

Which is caused by hardcoding the 'nova' path in the router. By removing 'nova/' from the router prefix the url is back to what one expects it to be, and the sidebar link works again.

Bad internal documentation links

Internal documentation links are also not handled properly probably because the package doesn't expect nova.path to end in /: image

Btw, href="//documentation/sample" generates: https://documentation/sample

Contents section links are not handled at all, probably because the regex didn't find what it expected: image

Home link broken in Samples page

In the samples page there's a link back to the home page clicking that hits a 404 (at /documentation/home)

Workaround?

Changing nova.path from '/' to an empty string '' fixes some of these issues, but ideally the package should handle URIs better.

dm-pf commented 1 year ago

@dniccum I am not sure that PR #27 by itself solves the entirety of what this ticket reports. Appreciated the quick action though!

dniccum commented 1 year ago

@dm-pf Ok thanks for the update. I will have some time today to dive into this a bit further.

dniccum commented 1 year ago

@dm-pf Please review the most recent release to see if this fixes your issue.

dm-pf commented 1 year ago

@dniccum most recent release fixes most of the issues indeed. https://github.com/dniccum/nova-documentation/issues/28 is still broken though, I'll submit a PR in a few minutes to properly fix that.

dniccum commented 1 year ago

This has been fixed with #34