jrief / django-formset

The missing widgets and form manipulation library for Django
https://django-formset.fly.dev/
MIT License
317 stars 30 forks source link

DualSelector regression on v1.3.10 #135

Closed florczakraf closed 2 months ago

florczakraf commented 4 months ago

I've just updated to version 1.3.10 from 1.3.8 and can't seem to get DualSelector to work anymore. I've also tested 1.3.9 -- it's still OK. I can see both sides of the selector rendered under the widget for a split second and then they are both empty:

image

Searching seems to work: image

but the elements can't be moved.

There's the following DOMException in the browser's console:

image

but I don't think it's related because there's a similar one on older versions of the library.

Some options are visible in DOM: image

There are no errors on the server side.

This is how I use the DualSelector:

    rivals = forms.models.ModelMultipleChoiceField(
        queryset=Player.objects.all(),
        widget=DualSelector(search_lookup=["name__icontains", "machine_tag__icontains"]),
        required=False,
    )

cross-ref: https://github.com/florczakraf/boogie-stats/issues/173

florczakraf commented 4 months ago

We've managed to narrow down the issue to the web browser version. The issue manifests on Firefox ESR 102 which is the previous LTS release of Firefox.

I think that there are multiple takeaways from this issue:

  1. this project could list a browser compatibility matrix
  2. all deprecations should be communicated in the release notes
  3. compatibility breaks should probably not happen on patch version bumps
florczakraf commented 4 months ago

There's a brief mention of the supported browsers on https://django-formset.fly.dev/installation/ page:

The provided JavaScript file is optimized for modern browsers, which can handle EcmaScript-ES2020, or later. These browsers are Chrome 94+, Edge 94+, Firefox 93+, Safari 15+ and Opera 81+. In the rare occasion that you have to support a legacy browser, choose an appropriate target from the TypeScript build options and recompile the sources.

jrief commented 4 months ago

For debugging purpose, it would be a good idea to compile the client code using the debug options. This does not minify the code and adds a Sourcemap.

jrief commented 4 months ago

Just tested my own examples of the DualSelector on FireFox 125.0.1 and I was unable to find any error. Isn't version 102 quite old now?

florczakraf commented 4 months ago

For debugging purpose, it would be a good idea to compile the client code using the debug options. This does not minify the code and adds a Sourcemap.

I'm happy to test and debug that, but please let me know how to prepare such build because I'm not familiar with front/js frameworks.

Isn't version 102 quite old now?

I believe it's not that old. The most recent release in this line was in March: https://ftp.mozilla.org/pub/firefox/releases/102.15.1esr/linux-x86_64/en-US/. It's also way above the specified 93+.

jrief commented 4 months ago

This should suffice to get the client up and running: https://django-formset.fly.dev/development/

If not, please tell me where you're stuck.

About version 102: In browser life cycles I would consider 2 years quite old.

florczakraf commented 4 months ago

I further narrowed the issue down to 9c44f7b0983093861ac2203eff2c03b77fa24ee8 where this.transferStyles() call has been moved to constructor, where an unhandled exception from DualSelector.ts:403:

image

        sheet.insertRule(`${this.baseSelector} .left-column:has(>input:focus, >select:focus){${extraStyles}}`, ++index);

the string evaluates to:

"django-formset [role=\"group\"] .dj-dual-selector .left-column:has(>input:focus, >select:focus){box-shadow:rgba(13, 110, 253, 0.25) 0px 0px 0px 4px; }"

It seems like the issue pretty much predates the mentioned commit but due to the fact that the call order has changed it manifested there.

The actual problem seem to be the use of :has() pseudo-class, which has gained consistent support across major browsers on 19 December 2023.

I tracked the changes to image which seem to be purely visual, not functional, so it should be safe to make them optional when the browser supports them.

jrief commented 4 months ago

FireFox has support for :has() since version [121](https://caniuse.com/?search=%3Ahas()).

Problem could be that I use these selectors in my E2E-tests and during style pilfering. Therefore it is good to know, that this project requires FF 121 or later.

Thanks for reporting.