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

Add dontUpdateUrl option to Router.navigate #155

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
8 changes: 7 additions & 1 deletion lib/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,12 @@ export interface AnyAttrs {
[attr: string]: any;
}

export interface NavigateOptions {
// Passing this flag to navigate() will cause the matching router handler to execute, but won't update the url
// with the passed fragment.
dontUpdateUrl?: boolean;
}

export class Component<
StateT,
AttrsT = AnyAttrs,
Expand Down Expand Up @@ -240,7 +246,7 @@ export class Component<
* Executes the route handler matching the given URL fragment, and updates
* the URL, as though the user had navigated explicitly to that address.
*/
navigate(fragment: string, stateUpdate?: Partial<StateT>): void;
navigate(fragment: string, stateUpdate?: Partial<StateT>, navigateOptions?: NavigateOptions): void;

/** Run a user-defined hook with the given parameters */
runHook: (
Expand Down
6 changes: 4 additions & 2 deletions lib/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,15 @@ export default class Router {
this.window.history[this.historyMethod] = this.origChangeStateMethod;
}

navigate(fragment, stateUpdate = {}) {
navigate(fragment, stateUpdate = {}, {dontUpdateUrl = false} = {}) {
Copy link
Author

Choose a reason for hiding this comment

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

I considered naming the flag updateUrl, but slightly prefer false by default param, so went with dontUpdateUrl

fragment = stripHash(fragment);
if (decodedFragmentsEqual(this.app.state.$fragment, fragment) && !Object.keys(stateUpdate).length) {
return;
}

stateUpdate.$fragment = fragment;
if (!dontUpdateUrl) {
stateUpdate.$fragment = fragment;
}
for (const route of this.compiledRoutes) {
const matches = route.expr.exec(fragment);
if (matches) {
Expand Down
8 changes: 4 additions & 4 deletions test/browser/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -321,15 +321,15 @@ describe(`Simple Component instance`, function () {

context(`when applying override styles`, function () {
it(`appends the overriding styles to the default styles`, async function () {
el.setAttribute(`style-override`, `:host { background: red; }`);
el.setAttribute(`style-override`, `:host { background-color: red; }`);
await nextAnimationFrame();
expect(getComponentStylesheetText(el)).to.equal(`:host { color: blue; }:host { background: red; }`);
expect(getComponentStylesheetText(el)).to.equal(`:host { color: blue; }:host { background-color: red; }`);
});

it(`it applies the styles even if the component isn't attached to the DOM`, function () {
el = document.createElement(`shadow-dom-app`);
el.setAttribute(`style-override`, `:host { background: red; }`);
expect(getComponentStylesheetText(el)).to.equal(`:host { color: blue; }:host { background: red; }`);
el.setAttribute(`style-override`, `:host { background-color: red; }`);
expect(getComponentStylesheetText(el)).to.equal(`:host { color: blue; }:host { background-color: red; }`);
Comment on lines +324 to +332
Copy link
Author

Choose a reason for hiding this comment

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

Fix test failure in Firefox: expected ':host { color: blue; }:host { background: red none repeat scroll 0% 0%; }' to equal ':host { color: blue; }:host { background: red; }'

Unrelated to my changes (happens on master).

});
});
});
Expand Down
8 changes: 8 additions & 0 deletions test/browser/router.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,14 @@ describe(`Router`, function () {
await nextAnimationFrame();
expect(this.routerApp.textContent).to.equal(`Number: 42`);
});

it(`does not update url if dontUpdateUrl is passed`, async function () {
expect(window.location.hash).to.equal(``);
this.routerApp.router.navigate(`numeric/42`, {}, {dontUpdateUrl: true});
await retryable(() => expect(this.routerApp.textContent).to.equal(`Number: 42`));
expect(this.routerApp.textContent).to.equal(`Number: 42`);
expect(window.location.hash).to.equal(``);
});
});

describe(`replaceHash()`, function () {
Expand Down