search

silentsokolov / django-admin-rangefilter

A Django app that lets you filter data by date range and numeric range in the admin UI
MIT License
713 stars 106 forks source link

Duplicate datetimeshortcuts for each input #128

Closed vxsx closed 1 month ago

vxsx commented 1 month ago

The datetimeshortcuts are rendered twice in (at least) Chrome. It's not visible because in default django admin they are rendered on top of each other, but I believe it's the same error that happens in https://github.com/silentsokolov/django-admin-rangefilter/issues/127

You can see the problem in the markup:

Screenshot 2024-08-14 at 09 38 31

This seems to happen in Chrome and not in Firefox because of a bug in jQuery (or in one of the browsers I guess). In chrome scripts injected in the $(document).ready(...) callback delay the load event and in firefox they don't. This package loads DateTimeShortcuts js file which has an event handler attached to window load event that initializes the widgets.

In chrome this happens after this callback

https://github.com/silentsokolov/django-admin-rangefilter/blob/474784027cf90a30a9ebe95fb120b5b80d0ecea9/rangefilter/templates/rangefilter/date_filter.html#L143-L147

which makes them initialize twice and in FF before (so the existing buttons are first removed and then initialized again)

One possible solution would be to not use jQuery to wait until document ready and do it via document.addEventListener("DOMContentLoaded", ...) and some additional changes to initialization or similar or wait till load event instead.

silentsokolov commented 1 month ago

I'm not an expert in JS. Do you think changing $(document).ready to document.addEventListener is a better way? Does it not create more bugs?

vxsx commented 1 month ago

Nowadays browsers are a lot better at handling more basic things and django admin browser support seems to be limited to fairly recent versions of browsers (https://docs.djangoproject.com/en/5.1/faq/admin/#what-browsers-are-supported-for-using-the-admin)

If you'd still prefer to use jQuery then a simple switch to django.jQuery(window).on('load', ...) would also work as that will make sure that code only runs after everything else is finished.

I can provide a merge request with a fix if you'd like.

silentsokolov commented 1 month ago

If you'd still prefer to use jQuery then a simple switch to django.jQuery(window).on('load', ...)

Thanks

I can provide a merge request with a fix if you'd like.

That would be great!