search

djk2 / django-tables2-column-shifter

Simple extension for django-tables2 can dynamically show or hide columns. Using JQuery, Bootstrap 3, Bootstrap 4, Bootstrap 5 and Django >=1.9
BSD 3-Clause "New" or "Revised" License
21 stars 8 forks source link

PR for issue 28 #33

Closed mpibpc-mroose closed 1 year ago

mpibpc-mroose commented 1 year ago

I wrote a proof of concept for implementing responsive tables for Bootstrap v4 and v5. Basically the goal was to use another base template from django-tables2. To make it DRY I needed to introduce some partials and move some of the code out of the templates for re-use.

Roughly tested it in my projects and it completely does fulfill my needs.

djk2 commented 1 year ago

I have also one general request. In repository exist also the testproject where you can in quick way launch demo project to test how library works. Can you also extend this demo project to show how lib works in "responsive" mode. In testproject on "index/home" page user can choice a version of bootstrap from 2 to 5. (blue buttons), I suggest add 2 more buttons for boostrap4-responsive and boostrap5-responsive What do you think ?

image

On this occasion you can also change a button layout from horizontal to vertical. You know :) when I started working on this library I had only 2 versions (buttons) of bootstrap. Now there is a five buttons - it will be better to display them in vertical stack.

djk2 commented 1 year ago

Can you also increase the version of library in django_tables2_column_shifter/__init__.py from 2.1.1 to 2.2.0 and add a note in CHANGELOG.rst describing changes for this version. ?

mpibpc-mroose commented 1 year ago

Should be all resolved now.

djk2 commented 1 year ago

Can you fix the tests? To run test locally you can execute command tox -r --workdir /tmp/tox-dir At now, tests returning error: 

django.template.exceptions.TemplateDoesNotExist: django_tables2/bootstrap4-responsive.html

To code I do not have more comments.

mpibpc-mroose commented 1 year ago

That's pretty interesting. I'm relying on a template from django-datatables2 which seemed to have introduced in the 2.5 versions (django_tables2/bootstrap{4,5}-responsive.html). I wasn't aware of this. And it makes it pretty difficult for me to decide how to proceed as I was building anything around that.

I'm pretty unsure how to proceed. Any advice?

djk2 commented 1 year ago

Give me some time to consider how solve it. I will try to look on that tomorrow

djk2 commented 1 year ago

That's pretty interesting. I'm relying on a template from django-datatables2 which seemed to have introduced in the 2.5 versions (django_tables2/bootstrap{4,5}-responsive.html). I wasn't aware of this. And it makes it pretty difficult for me to decide how to proceed as I was building anything around that.

I'm pretty unsure how to proceed. Any advice?

Hi. I added suggestions to code. Changes are not vast. I proposed to add a exclusions and inform users that they can use new features only for some of versions of django-tables2. I included changes in tests. You have also fix the pep8. 

./django_tables2_column_shifter/tests/urls.py:3:101: E501 line too long (109 > 100 characters)
./testproject/testproject/urls.py:21:101: E501 line too long (102 > 100 characters)
./testproject/testproject/urls.py:24:101: E501 line too long (102 > 100 characters)