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 widgetRemoveEnd event #1430

Closed
wants to merge 2 commits into from
Closed

Add widgetRemoveEnd event #1430

wants to merge 2 commits into from

Conversation

Fohlen
Copy link

@Fohlen Fohlen commented Jul 17, 2017

This PR introduces a widgetRemoveEnd event which can be quiet handy in the following set up

  • one has a widget that is occasionally added and removed (depending on what table is sorted)
  • the sort end event can happen after the widget has been removed and initialized
  • this can result in the old table being used for the widget

=> using a widgetRemoveEnd event makes it possible to intercept the above behavoir.
Sorry for the big diff. It's only 2 lines actually, but grunt rewrote about 97%, if you need to know the exact changes, let me know.

@Mottie
Copy link
Owner

Mottie commented Jul 17, 2017

Hi @Fohlen!

I can add ?w=1 to the url to ignore all the whitespace changes...

  1. Please put all my tabs back. I love my tabs!
  2. The testing function has this addition assert.equal( ts, 'removeWidgetEnd fired' );. I didn't try this, but ts is the variable containing the tablesorter object $.tablesorter. I don't see how that could be equal.

@Fohlen
Copy link
Author

Fohlen commented Jul 18, 2017

Hey @Mottie

  1. That must have been Atom! If you urge, I can convert spaces -> tabs again
  2. uh. I'm not really introduced into QUnit and didn't understand how it tests the events. However I verified that it works in Chrome. Mind updating the unit test?

@Mottie
Copy link
Owner

Mottie commented Jul 18, 2017

I think it might be easier for me to add this enhancement then... I'll get to it when I get some time, probably later this afternoon.

@Mottie Mottie closed this in 8c2f14b Aug 27, 2017
@Mottie
Copy link
Owner

Mottie commented Sep 28, 2017

Added in v2.29.0 - see docs.

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