Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

prepend http_prefix to links, if defined #9

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

prepend http_prefix to links, if defined #9

wants to merge 2 commits into from

Conversation

avillafiorita
Copy link

I have slightly changed your code to prepend the value of :http_prefix to the generated links. If :http_prefix is not defined the gem behaves as before.

This allows to use your gem on sites deployed on a suburi.

@marnen
Copy link
Owner

marnen commented May 30, 2016

Is this so it handles both HTTP and HTTPS links properly?

@marnen
Copy link
Owner

marnen commented May 30, 2016

Also, please don't change the version number. I'll do that when I release the gem.

@@ -21,7 +21,9 @@ def initialize(app, options_hash = {}, &block)
def breadcrumbs(page, separator: @separator, wrapper: @wrapper)
hierarchy = [page]
hierarchy.unshift hierarchy.first.parent while hierarchy.first.parent
hierarchy.collect {|page| wrap link_to(page.data.title, "/#{page.path}"), wrapper: wrapper }.join(h separator)
hierarchy.collect {|page|
wrap link_to(page.data.title, "#{File.join((app.config[:http_prefix] || "/"), page.path)}"),
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be better with URI.join.

@marnen
Copy link
Owner

marnen commented May 30, 2016

This should not be necessary at all. The URLs that this gem generates aren't full prefixed URLs; they're paths. So the browser should automatically use the same HTTP prefix as the current page. Do you have a use case where that doesn't work?

@avillafiorita
Copy link
Author

I don't seem to be able to use the gem when deploying to a suburi (e.g., http://www.example.com/home). In my opinion it is because the gem generates absolute URLs, that is, it prepends a "/" to all URLs. The generated URLs, therefore, end up pointing to the wrong location if a site lives on a subdirectory.

The way I understand middleman, this is one of the reasons config[:http_prefix] can be defined in config.rb to prepend a string to the generated URLs.

Anyhow, live example of the problem I run into lives at http://gasapp.me/help-staging/admin/getting-started.html.

If there is a way to solve the issue without changing the code, even better.

@marnen
Copy link
Owner

marnen commented May 30, 2016

I see. I will take a look at this in detail later.

Note that I will need tests before I merge this pull request.

@@ -1,3 +1,3 @@
module BreadcrumbsVersion
VERSION = '0.4.1'
VERSION = '0.4.2'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't change the version number. I'll do that when I make a release.

@avillafiorita
Copy link
Author

Hi -
I have set the version number to 0.4.1.
thanks
-a

@marnen
Copy link
Owner

marnen commented Dec 20, 2016

Thank you. This still needs tests, however.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants