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

Prevent sort2Hash from adding extraneous entries to browser history #1447

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

alexweissman
Copy link
Contributor

Addresses userfrosting/UserFrosting#712 by using window.location.replace to update the browser URL only, rather than window.location.hash, which modifies the browser history.

Before you merge, a few things:

  • I haven't written any automated tests for this, as I'm not really familiar with qunit. It seems to work fine in UserFrosting, including scenarios with multiple tablesorters on the same page;
  • It's possible that these additional history entries might be considered a "feature" for some users; in that case we should probably make this a configurable option for the sort2Hash widget;
  • I don't know if this will cause adverse reactions with other widgets/features that may rely on the hashed URL being added to the browser history.

Use `window.location.replace` to update the browser URL only, rather than `window.location.hash`, which modifies the browser history.
@Mottie
Copy link
Owner

Mottie commented Sep 1, 2017

Hi @alexweissman!

Thanks for the code... I've been really bad about writing unit tests for everything, mostly widgets, so don't worry.

I think I'll go ahead and merge this, but I'll add an option to allow the developer to set the behavior.

@Mottie Mottie merged commit 6fe2cdf into Mottie:master Sep 1, 2017
@Mottie
Copy link
Owner

Mottie commented Sep 1, 2017

Added in f0426f8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants