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

Path-based routing #52

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

Conversation

ScraM-Team
Copy link
Contributor

@ScraM-Team ScraM-Team commented Aug 17, 2020

(This addresses issue #51)

This PR adds Path-based routing to Flavour.

The existing Hash-based routing has been refactored into a HashRoutingStrategy.

Routing now redirects to a strategy. The strategy can be set to path-based via Routing.usePathStrategy().

PathRoutingStrategy implements the path-based routing.

Developers need to do 3 things:

  • Call Routing.usePathStrategy()
  • Set the base URI in index.html: <base href="https://frequal.com/tea-sampler/" />
  • Add this to web.xml: <error-page> <location>/index.html</location> </error-page>

There is one limitation at present:

  • html:link causes a page reload. Use event:click to avoid this for now.

@konsoletyper
Copy link
Owner

* Set the base URI in index.html: `<base href="https://frequal.com/tea-sampler/" />`
* html:link causes a page reload.  Use event:click to avoid this for now.

These issues are fatal and should be resolved before I can accept this PR

@ScraM-Team
Copy link
Contributor Author

* Set the base URI in index.html: `<base href="https://frequal.com/tea-sampler/" />`
* html:link causes a page reload.  Use event:click to avoid this for now.

These issues are fatal and should be resolved before I can accept this PR

Thanks for the feedback!

I will start working on the html:link fix. I may need advice but I will give it a shot.

The base URI is a standard requirement from similar frameworks. (See this page) Perhaps it can be simplified or automated, if the app is deployed at the root. If the app is not deployed at the root there needs to be some way to specify the app root, otherwise resources will be loaded relative to the path, which won't work for deep linking.

I will investigate both and let you know what I find.

@ScraM-Team
Copy link
Contributor Author

Quick update: The html:link support is almost ready. I will investigate automating base href support after that.

@ScraM-Team
Copy link
Contributor Author

html:link support is complete and added to the PR.

@ScraM-Team
Copy link
Contributor Author

I'm investigating how to reduce or eliminate the 'base href' requirement. I am testing changes that make it much easier:

  1. If your index.html is deployed at the root of the site, no action is required. the PathRoutingStrategy will automatically set the base href for you.
  2. If your index.html is deployed below the root (like /ts/index.html), pass the path like this: Routing.usePathStrategy("ts"). That's it!

No changes are required to index.html in either case.

Does this seem acceptable?

@ScraM-Team
Copy link
Contributor Author

The base href fix is checked in now too. No base href is needed in index.html, it is set by the call to Routing.usePathStrategy. The default is "/" if no argument is provided, or you can pass the path if it is different (like "ts" if the app is deployed at "/ts").

@ScraM-Team
Copy link
Contributor Author

@konsoletyper , all of your concerns have been addressed. Could you please merge this? I have been using it for weeks on several Flavour projects and it seems stable.

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.

2 participants