search

alphagov / tech-docs-gem

Gem to distribute the tech docs project
https://tdt-documentation.london.cloudapps.digital/
MIT License
15 stars 38 forks source link

Update multi-page navigation to allow custom path prefix #136

Closed alex-ju closed 4 years ago

alex-ju commented 4 years ago

Currently if the tech-docs is deployed on a path (e.g. https://alphagov.github.io/api-catalogue/) the navigation links are broken as they are generated from root (/). One way to avoid this without affecting current single-page or multi-page configuration is to define a path_prefix in config that is prepended to each link.

This is an attempt to fix #128 which is currently blocking any deployment for the api-catalogue.

I'm open to any other suggestions that will unblock this - especially since I'm not familiar with this project or Middleman.

Note: I needed to refactor the conditional statements as linter was complaining about the code having too many lines (Metrics/BlockLength: Block has too many lines. [31/25])

alex-ju commented 4 years ago

Could we add some tests for this?

I thought a lot about that, but not sure where it fits as the test suite currently relies on one configuration file. This change will require a separate config and not sure where to accommodate that. Open to suggestions.

alex-ju commented 4 years ago

I don't see how this differs to http_prefix 🤔 and if http_prefix isn't doing what we want, it's probably a bug in its implementation and we should fix that rather than add a new property.

Ha, I haven't thought about that - I saw it as more of a way to specify the protocol (http/https). I'll give it a go, if it doesn't work I would really appreciate a hand, thank you!

ChrisBAshton commented 4 years ago

I agree the naming could be better!

alex-ju commented 4 years ago

As mentioned in the comments – this seems to have been addressed already.

alex-ju commented 4 years ago

This is still a problem – maybe https://github.com/alphagov/tech-docs-gem/pull/84 will fix it.