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

Is there a way to reinitialise / destroy instance of Filtering? #65

Open
squidgemonster opened this issue Jun 9, 2024 · 7 comments
Open

Comments

@squidgemonster
Copy link

Hey @robertpainsi - filtering.js has really helped me apply some very complex filters to a hobby project I'm working on - thanks for creating this :)

The only issue I have come across so far, is not quite understanding how to destroy / reinitialise filtering.js. I use the UI method.

My scenario: Currently I have a fairly large set of data that is reloaded each time a particular tab is visited... filtering.js works great on initial load, but seems to have issues when I revisit the data list (when it is reloaded). Performance seems to slow down, and the filters don't appear to work some of the time.

I suspect if I destroyed any instances of previous filters, or if could reset them in some way, when the data is refreshed, and could reinitialised the filters once more, the experience would be exactly the same as on first load.

Any help or suggestions would be very much appreciated.

Many thanks!

@squidgemonster
Copy link
Author

Any guidance would be really appreciated 🙏

@robertpainsi
Copy link
Owner

robertpainsi commented Jun 24, 2024

Thank you very much for submitting this issue! That's definitely sounds like a filtering.js bug we need to fix.

From you current description, I'm guessing you call new FilteringFlow(element) multiple times. Since FilteringFlow doesn't check if it has already been initialized on this element, this drastically reduces performance because filtering will be done multiple times now when clicking on a single filter.

As you suggested, destroying the FilteringFlow is the way to go here. For a simple fix, I'm thinking that we just remove all event listeners added by FilteringFlow and also set all it's properties to null. That way there won't be any memory leaks and the garbage collector can do it's thing ^^ Added and removed classes by FilteringFlow will be unaffected because developers can easily restore these themselves.

I deployed version 1.0.16 to npm and also updated all the source files here on GitHub. You can test it by calling destroy() on your FilteringFlow object. Updated documentation will follow 😅

let myFlow = new FilteringFlow(element);

myFlow.destroy();

Please let me know if this fixes your issue 🤞

@squidgemonster
Copy link
Author

Thank you so much for doing that @robertpainsi 🎉 That sounds like a great addition to the library. I was indeed noticing steady decline in performance each time the filtered list was reloaded in the page.

It may take a few days until I can give this a try - I will let you know how I get on though, as I believe this was the final piece in the puzzle.

I can pop you a link to the project when it's done too - as this has really helped make a looooong list of items a lot easier to filter. Thanks again!

@squidgemonster
Copy link
Author

squidgemonster commented Jun 29, 2024

Hi @robertpainsi - either my limited Javascript knowledge is failing me, or sadly the new destroy() method isn't quite right for my scenario.

The detailed sequence in which things happen on my app are:

  • User navigates to a UI tab
  • A click event on the tab triggers a list of items to be loaded with relevant filter data attributes
  • The loaded items are injected into a tag (#communityFilterRoot)
  • I apply the let myFlow = new MyFilterFlow(document.querySelector('#communityFilterRoot'));
  • Users can filter and everything works perfectly on first load...

The issue comes where if the user clicks the tab again, at which point the #communityFilterRoot tag is cleared and the items are once more downloaded and injected... but can't reference the original instance of myFlow at this point, and as you pointed out, it triggers another instance of myFlow in the process!

The ideal would be to be able to target destroy() at the point before the list of objects is loaded in, when the click event is first triggered - so I can clear any potential instances of filtering running against the #communityFilterRoot element - if that makes sense?

I used a library previously that let the destroy method be targeted to the element verses the instance, for example:

document.querySelector("#communityFilterRoot").filtering.destroy();

Would something similar be a possibility with filtering.js? Or I could have mis-understood how to apply the destroy() method you've recently so kindly added...?

@squidgemonster
Copy link
Author

If you're curious to see my implementation (I've removed the reload for the time being so I could deploy the new filter features):

https://www.hquestbuilder.com/?community=shinyfilters

Just click the filter icon to see the number of filters in play! :)

Thanks again for your work on this library @robertpainsi - as you can hopefully see, it's made a world of difference to allow filters on that list!

@robertpainsi
Copy link
Owner

robertpainsi commented Jul 1, 2024

Would something similar be a possibility with filtering.js? Or I could have mis-understood how to apply the destroy() method you've recently so kindly added...?

I'll need to do a little research here first. This approach is often used in JQuery as well but they don't expand/change a build-in object, in our case a HTMLElement. I'm just a little cautious here because once that option is in, it can't be removed easily, if at all, because developers will rely on it…

btw, you can assign the myFlow object to your communityFilterRoot HTMLElement already

const communityFilterRootElement = document.querySelector('#communityFilterRoot');
if (communityFilterRootElement.filtering) {
    communityFilterRootElement.filtering.destroy()
}
let myFlow = new MyFilterFlow(communityFilterRootElement);
communityFilterRootElement.filtering = myFlow;

or alternatively use a global variable.

if (window.myCommunityFilteringFlow) {
    window.myCommunityFilteringFlow.destroy()
}
let myFlow = new MyFilterFlow(document.querySelector('#communityFilterRoot'));
window.myCommunityFilteringFlow = myFlow;

Please let me know if that fixes your problem.

https://www.hquestbuilder.com/?community=shinyfilters

Holy Moly! Your app is amazing! Wow! All those details shows that you're passionate creating it! ❤️ 🚀

@squidgemonster
Copy link
Author

Hey :) thank you SO much for the help there... that fixed it! I now can destroy and reinitialise without any performance issues! Perfect! Problem very much fixed! ❤️

Thank you for your kind words about the app too. It has been a labour of love for several years now - it's lovely to see what the community have created using it over that time. Filtering.js now makes it so much easier for folks to find a quest they want to play now too 🙌

I've made sure you have a credit on the information screen and within the source-code 🤓 Your work has helped so much. I need to work on updating my minification task now, as it seems to get stuck on the # identifiers in filtering.js for some reason.

I have had another thought about a feature for AND vs OR on multiple tags on one item... but I'll create another ticket to discuss that I think...

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

No branches or pull requests

2 participants