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

Fix/navigation with root #41

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

Conversation

rluba
Copy link

@rluba rluba commented Jun 1, 2018

This is part of a patch for both aurelia-history-browser and aurelia-router and contains two bugfixes related to having root set to something else than /.

1. History could not handle outbound links

If root is set, eg., to /app1, the following link causes a routing error instead of navigating away from the app: < a href="/app2">Outbound link</a>.

This is fixed in 407959e.

2. History did not handle absolute paths within the app correctly

The router ignored root when creating URLs. This caused all non-default click actions (eg. CMD+Click, Right-click-> "Open in new Tab/Browser", etc.) to navigate to the wrong URL. (And broke links for crawlers.)

But as soon as the router creates correct URLs, history’s lack of proper root support causes all navigations to fail. This is fixed in bfad456.

@CLAassistant
Copy link

CLAassistant commented Jun 1, 2018

CLA assistant check
All committers have signed the CLA.

@EisenbergEffect
Copy link
Contributor

@davismj Can you review this please?

@EisenbergEffect EisenbergEffect requested a review from davismj June 13, 2018 04:25
@davismj davismj self-assigned this Aug 1, 2018
@davismj
Copy link
Member

davismj commented Aug 1, 2018

If root is set, eg., to /app1, the following link causes a routing error instead of navigating away from the app: Outbound link.

I need to play around with this in a live application, specifically with push state and non push-state and make sure that the behaviors match. I'm guessing the assessment is correct but I do want to double check.

History did not handle absolute paths within the app correctly

I've recently made some changes that affect the way links are generated. I have a feeling this issue might be fixed.

@rluba
Copy link
Author

rluba commented Aug 18, 2018

Let me know if I can help you in any way. I’ve had my changes to router and history running in production in a push-state app (with root prefix) for a while without issues.

@3cp
Copy link
Member

3cp commented Jan 8, 2019

For outbound link, use < a href="/app2" router-ignore>Outbound link</a> to bypass router.

@rluba
Copy link
Author

rluba commented Jan 8, 2019

@huochunpeng Your suggestion would require me to annotate all links that might ever be part of any page, including user-added content. Breaking regular HTML is not an option.

@3cp
Copy link
Member

3cp commented Jan 8, 2019

I see. For user created content, I guess probably we can support something like this.

<div class="router-ignore">
  <a></a>
</div>

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.

5 participants