search

yourlabs / django-autocomplete-light

A fresh approach to autocomplete implementations, specially for Django. Status: v4 alpha, v3 stable, v2 & v1 deprecated.
https://django-autocomplete-light.readthedocs.io
MIT License
1.79k stars 467 forks source link

Two isolated unadorned simple test cases added #1287

Closed bernd-wechner closed 2 years ago

bernd-wechner commented 2 years ago

Something sorely lacking from the test_project seems to be a really stripped down, isolated test of the single and multiselect widgets. Very useful and important, I believe, for seeing the very bare minimum configuration for a working widget without a lot of very distracting dressing in templates and views. These a totally drab new pages to demonstrate the stripped down bare minimalist widgets in action.

That said, something is wrong, and this first cut serves as little more than a minimal, stripped down demonstration of the focus bug being discussed (https://github.com/yourlabs/django-autocomplete-light/issues/1283). It is either a DAL bug or an incorrect use of DAL, but this tiny little view will help anyone assessing it to offer commentary on what has gone wrong. There's almost nothing there and hence it is, as per the goal, free of a lot of distracting context.

This is not a complete and form Pull Request, much rather a request for commentary. I do believe there is room for two such supremely simple test cases in the project, BUT of course there may be more enlightened or better implementations (or locations in the file system) of it. I call this a first cut for commentary and excellently demonstrating the bug mentioned (which may in face be a usage error in and this test illustrates an implementation that reproduces it).

bernd-wechner commented 2 years ago

I should add the help struck me as very dated and the way to try this PR at home is:

  1. Clone it: https://github.com/bernd-wechner/django-autocomplete-light
  2. Check out this branch: cd django-autocomplete-light then git checkout isolated-tests
  3. Build the venv: python -m venv your_venv_dir then source your_venv_dir/bin/activate and pip install -r requirements.txt
  4. Build the test database: python manage.py makemigrations then python manage.py migrate
  5. Run the test server python manage.py runserver

The two isolated tests are listed on the home page.

The first one is broken. And I (and I presume some others) would like to understand why and how to fix it.

bernd-wechner commented 2 years ago

So, given the quiet here on this (and on issue https://github.com/yourlabs/django-autocomplete-light/issues/1283) and that it has broken data entry on our site, I have taken steps to encourage and facilitate any useful diagnostic and remedial input. The test project for DAL can now be played with live at:

https://dal-test.thumbs.place/

And the Issue https://github.com/yourlabs/django-autocomplete-light/issues/1283 clearly seen at:

https://dal-test.thumbs.place/dal_single/

In a completely minified unadorned context to simplify diagnostics and comparisons (that site implements this PR of course).

So I would like to beg, plead, ask kindly, for any helpful input in fixing this site breaking issue introduced by recent upgrades.

bernd-wechner commented 2 years ago

This PR is now fixed and both isolated test work as intended and can serve as great minimalist reference for anyone who has a dysfunctional widget. Revealing the JS and CSS libraries that work!

jpic commented 2 years ago

Thanks for your contribution!

Please don't leave commented code, otherwise leave a comment to tell /why/ it's left commented ;)

Also, please 88 caracters per line max

bernd-wechner commented 2 years ago

Also, please 88 caracters per line max

Thanks for the tips and responding. I've now implemented an auto PEP8 formatter on my IDE for this project and pushed an update to the PR.

One issue I note is that the auto-formatter leaves string literals and comments longer than 88 chars. Is that kosher PEP8?

I'm not fussed, it's just a tad clunky to force HTML tag string literals to wrap, as the wrap is then part of the string. One way around it is to break the string into bits and add them. And can be done, but doesn't add to readability. Not that I'm fussed, if the project demands PEP8, PEP8 is what it gets. Just checking what the standards are (as I don't typically use PEP 8, preferring the PyDev default auto-formatter, which is a little neater IMHO - but clearly doesn't do 88 char line limits - which as a total aside puzzle me in 2022, as I haven't seen code in any context that makes 88 char limits attractive, screens are all wide, and viewports all seem to pop up horizontal scroll bars as needed - but it is not mine to judge what a mature project adopts as its standards, I abide! and PEP8 it is!).