Skip to content

[FIX] Handle hash change in url #337

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

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions app/instance-initializers/hash-to-query-redirect.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import hashToQuery from 'hash-to-query';

export function initialize(application) {
if (typeof FastBoot === 'undefined') {
window.onhashchange = function() {
let router = application.lookup('service:router');
let redirectToUrl = hashToQuery(window.location.hash, window.location.pathname);
if (redirectToUrl) {
router.transitionTo(redirectToUrl);
}
}
}
}

export default {
name: 'hash-to-query-redirect',
initialize
};
31 changes: 3 additions & 28 deletions lib/hash-to-query/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,33 +8,8 @@ module.exports = {
return true;
},

contentFor (type, config) {
Copy link
Contributor

Choose a reason for hiding this comment

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

is this really going to work for direct url access, such as say I have a bookmarked link for something with #method_toString?

Copy link
Contributor

Choose a reason for hiding this comment

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

if we are taking out the need for contentFor, then this doesn't need to be an in-repo addon. just make it a util function.

if (type === 'body') {
return `
<script type='text/javascript'>
var hash = window.location.hash;
if (hash && hash.length > 1) {
var pathName = window.location.pathname;
if (pathName.indexOf('/methods/') === -1 &&
pathName.indexOf('/properties/') === -1 &&
pathName.indexOf('/events/') === -1) {
var type = hash.slice(1, hash.indexOf('_'));
var name = hash.slice(hash.indexOf('_') + 1, hash.length);
var anchor = '?anchor=' + name + '&show=inherited,protected,private,deprecated';
var newPath = pathName;
if (type === 'method') {
newPath = pathName + '/methods/' + name;
} else if (type === 'property') {
newPath = pathName + '/properties/' + name;
} else if (type === 'event') {
newPath = pathName + '/events/' + name;
}
window.location.href = window.location.origin + newPath + anchor;
}
}
</script>
`
}
return '';
included(app) {
this._super.included.apply(this, arguments);
this.import('vendor/hash-to-query.js');
}
};
35 changes: 35 additions & 0 deletions lib/hash-to-query/vendor/hash-to-query.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
(() => {
/* globals define */
function hashToQuery(hash, pathName) {
if (hash && hash.length > 1) {
if (pathName.indexOf('/methods/') === -1 &&
pathName.indexOf('/properties/') === -1 &&
pathName.indexOf('/events/') === -1) {
let type = hash.slice(1, hash.indexOf('_'));
let name = hash.slice(hash.indexOf('_') + 1, hash.length);
let anchor = '?anchor=' + name + '&show=inherited,protected,private,deprecated';
let newPath = pathName;
if (type === 'method') {
newPath = pathName + '/methods/' + name;
} else if (type === 'property') {
newPath = pathName + '/properties/' + name;
} else if (type === 'event') {
newPath = pathName + '/events/' + name;
}
return newPath + anchor;
}
}
}
if (typeof FastBoot === 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

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

what's this for? Should never enter this if I don't think...

let redirectToUrl = hashToQuery(window.location.hash, window.location.pathname);
if (redirectToUrl) {
window.location.href = window.location.origin + redirectToUrl;
}
}
define('hash-to-query', function () {
'use strict';
return {
default: hashToQuery
}
});
})()
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
"ember-power-select": "^1.9.2",
"ember-resolver": "^4.3.0",
"ember-route-action-helper": "^2.0.5",
"ember-router-service-polyfill": "1.0.1",
"ember-service-worker": "0.6.7",
"ember-service-worker-asset-cache": "0.6.1",
"ember-service-worker-cache-fallback": "0.6.1",
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/initializers/hash-to-query-redirect-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import Ember from 'ember';
import { initialize } from 'ember-api-docs/instance-initializers/hash-to-query-redirect';
import { module, test } from 'qunit';
import destroyApp from '../../helpers/destroy-app';
import hashToQuery from 'hash-to-query';

module('Unit | Instance Initializer | hash to query redirect', {
beforeEach() {
Ember.run(() => {
this.application = Ember.Application.create();
this.appInstance = this.application.buildInstance();
});
},
afterEach() {
Ember.run(this.appInstance, 'destroy');
destroyApp(this.application);
}
});

// Replace this with your real tests.
test('it works', function(assert) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to have some input/output tests for the hashToQuery function here. (unless the test coverage for it is elsewhere already)

Copy link
Contributor

Choose a reason for hiding this comment

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

Simple acceptance test would be cool as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added tests for hashToQuery

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to verify that legacy links with hashes work (as the currently do today)

initialize(this.appInstance);

// you would normally confirm the results of the initializer here
assert.ok(true);
});

test('hashToQuery method works', function(assert) {
initialize(this.appInstance);

let redirectToUrl = hashToQuery('method', '/class');
assert.equal(redirectToUrl, '/class?anchor=method&show=inherited,protected,private,deprecated', 'hashToQuery function works');
Copy link
Contributor

@MartinMalinda MartinMalinda Sep 7, 2017

Choose a reason for hiding this comment

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

try these tests with some real data scenarios:

let redirectToUrl = hashToQuery('#method', '/class');
  assert.equal(redirectToUrl, '/class?anchor=method&show=inherited,protected,private,deprecated', 'hashToQuery function works');
  assert.equal(hashToQuery('#method_pushPayload', '/ember-data/2.14/classes/DS.Store'), '/ember-data/2.14/classes/DS.Store/methods/pushPayload?anchor=pushPayload&show=inherited,protected,private,deprecated');
  assert.equal(hashToQuery('#method_pushPayload', '/ember-data/2.14/classes/DS.Store/methods/createRecord'), '/ember-data/2.14/classes/DS.Store/methods/pushPayload?anchor=pushPayload&show=inherited,protected,private,deprecated');
  assert.equal(hashToQuery('#method_push', '/ember-data/2.14/classes/DS.Store/methods/createRecord'), '/ember-data/2.14/classes/DS.Store/methods/push?anchor=push&show=inherited,protected,private,deprecated');

Only the second one passes. The others fail because they contain /methods/. Either the hashToQuery function needs to be updated or a new function needs be made that shares some of the logic.

});