PR for issue 28 #33

mpibpc-mroose · 2023-04-13T11:19:17Z

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.

…ap v4 and v5. Fixes djk2#28.

...column_shifter/templates/django_tables2_column_shifter/_partials/bootstrap4_table_block.html

...go_tables2_column_shifter/templates/django_tables2_column_shifter/bootstrap4-responsive.html

...column_shifter/templates/django_tables2_column_shifter/_partials/bootstrap4_table_block.html

djk2 · 2023-04-18T10:15:08Z

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 ?

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 · 2023-04-18T10:22:37Z

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. ?

Co-authored-by: K2 <[email protected]>

mpibpc-mroose · 2023-04-19T10:46:45Z

Should be all resolved now.

djk2 · 2023-04-26T12:41:22Z

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 · 2023-04-27T14:02:53Z

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 · 2023-04-27T15:01:23Z

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

django_tables2_column_shifter/tests/test_base.py

CHANGELOG.rst

README.rst

django_tables2_column_shifter/tests/test_base.py

django_tables2_column_shifter/tables.py

djk2 · 2023-04-28T08:07:08Z

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)

Co-authored-by: K2 <[email protected]>

…es2-column-shifter # Conflicts: # CHANGELOG.rst # README.rst # django_tables2_column_shifter/tables.py # django_tables2_column_shifter/templates/django_tables2_column_shifter/_partials/bootstrap4_table_block.html # django_tables2_column_shifter/templates/django_tables2_column_shifter/bootstrap4-responsive.html # django_tables2_column_shifter/tests/test_base.py # django_tables2_column_shifter/tests/urls.py

djk2 · 2023-04-29T07:50:28Z

django_tables2_column_shifter/tests/test_base.py

@@ -12,7 +12,9 @@
    ColumnShiftTableBootstrap2,


Please import dt_version from django_tables2_column_shifter.tables you added it there.

djk2 · 2023-04-29T07:51:08Z

django_tables2_column_shifter/tests/test_base.py

+            'max_dt_version': None,
+            'template_name': 'django_tables2_column_shifter/bootstrap5-responsive.html',
+            'table_clsss': ColumnShiftTableBootstrap5Responsive,
+        },
    ]

    dt_version = tuple(map(int, tables.__version__.split(".")[:2]))


Can you remove dt_version from here and import it from django_tables2_column_shifter.tables at top of file.
After yesterday changes we already have dt_version in tables
Moreover, now you need version containing 3 digits instead 2.

…nor and sub-minor version.

djk2 · 2023-05-02T16:06:13Z

django_tables2_column_shifter/tests/test_base.py

 )
+from django_tables2_column_shifter.tables import dt_version


Put this import between 11 and 12 line.
Above package tables is already imported from django_tables2_column_shifter

Your absolutely right!

mpibpc-mroose added 7 commits April 12, 2023 14:58

Added extra table classes to support responsive tables. Fixes djk2#28.

6f92ac7

Added extra table classes to support responsive tables. Fixes djk2#28.

a7e676a

Added extra table classes to support responsive tables. Fixes djk2#28.

4825cab

Added extra table classes to support responsive tables. Fixes djk2#28.

e67c49e

Added extra table classes to support responsive tables. Fixes djk2#28.

42ae163

Added extra table classes to support responsive tables. Fixes djk2#28.

a6ff3ab

Implemented extra classes for support of responsive tables in Bootstr…

9fdf2a8

…ap v4 and v5. Fixes djk2#28.

djk2 requested changes Apr 18, 2023

View reviewed changes

mpibpc-mroose and others added 7 commits April 19, 2023 11:21

remove whitespace

cd52c41

Co-authored-by: K2 <[email protected]>

Remove blank line

927374c

Co-authored-by: K2 <[email protected]>

Remove blank line

ab9feb8

Co-authored-by: K2 <[email protected]>

Removed white space.

48684c2

Co-authored-by: K2 <[email protected]>

Bumped version to 2.2.0 and documented changes.

6eb9cb2

Added responsive versions of the tables to the test project.

db051e4

Changed the layout of the table choose buttons in the test project.

3d324e7

mpibpc-mroose requested a review from djk2 April 19, 2023 10:47

djk2 requested changes Apr 28, 2023

View reviewed changes

mpibpc-mroose and others added 6 commits April 28, 2023 10:36

Update django_tables2_column_shifter/tests/test_base.py

0c531d1

Co-authored-by: K2 <[email protected]>

Update django_tables2_column_shifter/tests/test_base.py

c5d0a36

Co-authored-by: K2 <[email protected]>

Update django_tables2_column_shifter/tests/test_base.py

0aeb702

Co-authored-by: K2 <[email protected]>

Update django_tables2_column_shifter/tables.py

fd34282

Co-authored-by: K2 <[email protected]>

Update django_tables2_column_shifter/tables.py

d1fd149

Co-authored-by: K2 <[email protected]>

Update django_tables2_column_shifter/tables.py

83517ad

Co-authored-by: K2 <[email protected]>

mpibpc-mroose and others added 8 commits April 28, 2023 10:46

Update django_tables2_column_shifter/tables.py

e4273c1

Co-authored-by: K2 <[email protected]>

Solved linting issue.

a8ca391

Bumped version.

79ad1a1

Fixed whitespace.

1451786

Added description about responsive table usage.

783bb87

Solved linting issues.

1d3775a

Re-defined test cases.

f5e0397

mpibpc-mroose requested a review from djk2 April 28, 2023 14:14

djk2 requested changes Apr 29, 2023

View reviewed changes

Changed import of dt_version. Updated test cases to support major, mi…

e7175c5

…nor and sub-minor version.

djk2 reviewed May 2, 2023

View reviewed changes

Moved import of dt_version.

adb07a3

djk2 merged commit 92a49e6 into djk2:master May 3, 2023
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR for issue 28 #33

PR for issue 28 #33

mpibpc-mroose commented Apr 13, 2023

djk2 commented Apr 18, 2023 •

edited

Loading

djk2 commented Apr 18, 2023

mpibpc-mroose commented Apr 19, 2023

djk2 commented Apr 26, 2023

mpibpc-mroose commented Apr 27, 2023

djk2 commented Apr 27, 2023

djk2 commented Apr 28, 2023

djk2 Apr 29, 2023

djk2 Apr 29, 2023

djk2 May 2, 2023

mpibpc-mroose May 3, 2023

		)
		from django_tables2_column_shifter.tables import dt_version

PR for issue 28 #33

PR for issue 28 #33

Conversation

mpibpc-mroose commented Apr 13, 2023

djk2 commented Apr 18, 2023 • edited Loading

djk2 commented Apr 18, 2023

mpibpc-mroose commented Apr 19, 2023

djk2 commented Apr 26, 2023

mpibpc-mroose commented Apr 27, 2023

djk2 commented Apr 27, 2023

djk2 commented Apr 28, 2023

djk2 Apr 29, 2023

Choose a reason for hiding this comment

djk2 Apr 29, 2023

Choose a reason for hiding this comment

djk2 May 2, 2023

Choose a reason for hiding this comment

mpibpc-mroose May 3, 2023

Choose a reason for hiding this comment

djk2 commented Apr 18, 2023 •

edited

Loading