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

feat(history): add getOrigin method #7

Merged
merged 1 commit into from May 9, 2016

Conversation

jwahyoung
Copy link
Contributor

Per aurelia/router#88 we want to be able to generate an absolute URI with the router.generate() method. That method relies upon the history module. getOrigin allows the router to generate an absolute URI with the protocol, hostname, and port.

* Returns the fully-qualified root of the current history object.
*/
getAbsoluteRoot(): string {
throw new Error('History must implement getBaseUrl().');
Copy link
Contributor

Choose a reason for hiding this comment

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

Error message doesn't match method name

@EisenbergEffect
Copy link
Contributor

@jedd-ahyoung If you can address the comment from @bryanrsmith then pending any other discussion we can probably merge this.

@jwahyoung
Copy link
Contributor Author

This has been updated with a corrected error message.

@EisenbergEffect
Copy link
Contributor

@bryanrsmith I'll let you decide when to merge this since there are several other PRs that are part of the same logical improvement.

@EisenbergEffect
Copy link
Contributor

Where are we at with this? We want to get to an RC, so I'd like to get things like this resolved in the next couple of weeks.

@jwahyoung
Copy link
Contributor Author

@EisenbergEffect, I will look into this personally in the coming week. This
hasn't moved due to my lack of time. I may be able to schedule some time to
work on this specifically with @bryanrsmith.

2016-03-19 12:41 GMT-04:00 Rob Eisenberg notifications@github.com:

Where are we at with this? We want to get to an RC, so I'd like to get
things like this resolved in the next couple of weeks.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#7 (comment)

@EisenbergEffect
Copy link
Contributor

@jedd-ahyoung ping

@EisenbergEffect
Copy link
Contributor

After the modal stuff is done, we really need to get this wrapped up fast.

@EisenbergEffect
Copy link
Contributor

Status?

We need this and dialog issues wrapped in the next week or two at most. You weren't in the meeting...but we are trying to get to RC and this is holding us up....

Per aurelia/router#88 we want to be able to generate an absolute URI with the `router.generate()` method. That method relies upon the history module. `getAbsoluteRoot` allows the router to generate a fully-qualified root URL.
@jwahyoung
Copy link
Contributor Author

@EisenbergEffect This can be merged, along with aurelia/history-browser#22 and aurelia/router#294. The test is failing because of a missing karma entry in package.json. Previously, I think we had an issue with the hash or something like that - something about options.root - but I can't find the original problem, and it looks as though that's been resolved. Testing this in the skeleton-navigation app yields positive results.

@EisenbergEffect
Copy link
Contributor

Ok, great. @bryanrsmith Do you have time to do a quick second look?

@bryanrsmith
Copy link
Contributor

lgtm

@EisenbergEffect EisenbergEffect merged commit 96b5e6e into aurelia:master May 9, 2016
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.

None yet

3 participants