Issue #265 - user panel control #288

Naltharial · 2012-09-10T11:19:40Z

#265: Added basic support for users to select which panels they should show. Dependencies checked on form validation.

TODO: Required/recommended extension for inter-panel dependencies.

…e homepage panels.

mitar · 2012-09-12T04:29:07Z

piplmesh/panels/dummy_test/panel.py

@@ -0,0 +1,22 @@
+import random


Why additional panel? You could just in original panel file extend another panel class and register it. To reduce duplication. So, use inheritance.

…to Issue wlanslovenija#278.

Naltharial · 2012-09-13T13:44:17Z

Changed the form to metaclass, changed the Panels model. Not sure why Issue #278 has a double-nested MapField, however. Or why we're passing number_of_columns as an argument. So right now, each user has a list of their panels and every panel can be collapsed and is at exactly one (column,order) position.

mitar · 2012-09-14T10:32:06Z

Based on the width of the window, number of columns for panels change. So we have double nested mapfiled because we are storing layout of panels for each number of columns there is. So once we map panel id to Panel. And then we map number of columns to PanelState, which tells where for this number of columns panel is.

mitar · 2012-09-14T10:32:46Z

So you will have to fix this. :-)

Naltharial · 2012-09-16T00:55:08Z

Added the second MapField. I've just pulled collapsed out of PanelLayout, because it's not connected to number of columns.

mitar · 2012-09-16T00:56:43Z

Not true. It can be. If you have very narrow window on your laptop, you might not want to display all panels. You do not want them to be removed, but just collapsed. On your big monitor on your PC, you have more columns and you want all of panels to be shown fully.

Naltharial · 2012-09-16T01:08:18Z

Well, sure, it just wasn't connected currently the way the JavaScript part is set up (requests to panel/collapse/ do not include number_of_columns). I can connect it, but that would mean changing the JavaScript behind it, which I thought was part of another issue.

mitar · 2012-09-16T01:13:12Z

OK. You decide. So I am explaining why model was proposed in the way it was. It is true that current code does not behave like that. But the end it should. So you can fix it yourself, or add TODO/ticket for this.

Naltharial · 2012-09-16T17:52:40Z

Added the number to the requests for collapse and changed the model.

mitar · 2012-09-19T08:45:59Z

piplmesh/account/forms.py

+ """
+ Form for selecting homepage panels.
+ """
+ __metaclass__ = PanelFormMetaclass


Empty line after docstring.

Naltharial · 2012-10-14T14:10:11Z

I've replaced the get_columns method, the frontend does seem to work even with new panels that haven't been ordered yet. I had to use panels_pool in view though, because otherwise you don't get dependencies for panels that aren't currently selected.

Test cases also seem to pass now.

mitar · 2012-10-15T13:02:27Z

piplmesh/account/models.py

+ """
+ This class holds panel instances for a user, their properties and layouts.
+
+ :param dict layout: mapping of count of displayed columns (`string`) to `PanelState` objects for that count


Now I noticed. This is not really a param. You use this when commenting functions and which parameters they take. In this case this is a class. and even class constructor does not take this parameter.

You might simply move this line above layout definition bellow and just leave it as a in-code-only comment.

mitar · 2012-10-15T13:32:35Z

the frontend does seem to work even with new panels that haven't been ordered yet.

Yes. This is good. So probably only frontend should decide how to order new panels. Not backend

I had to use panels_pool in view though, because otherwise you don't get dependencies for panels that aren't currently selected.

I agree. But we had get_all_panels method or something on user object before, why is that one not good? It can return for now the same list as panels_pool. But later on it could be limited (by, for example, permissions (debug panel could be available only to developers), or payment (we could have some panels available only if you buy them)).

mitar · 2012-10-15T19:08:43Z

Feature-wise, things are good now. So just check all your code for possible tabs and fix indentations and this is ready to go in.

There is one minor detail you can do, if you want, or open a ticket. You have to enable for elements before submit. But this is ugly as it changes user feedback and maybe confuses her. So some other solution should be used. What Internet recommends? Maybe a hidden input could be inserted which is kept in sync with checkbox? Maybe each checkbox could have its own hidden input all the time, kept in sync. JavaScript could rename all checkboxes and add hidden input box instead with that form name.

Naltharial · 2012-10-22T10:37:02Z

I've added a second set of hidden inputs, so we don't have to un-disable the checkboxes. They're IntegerField, because hidden checkboxes don't properly submit state.

mitar · 2012-10-22T13:23:08Z

piplmesh/account/forms.py

+ data_box = panel.get_name()
+ display_box = data_box + "_display"
+
+ attrs[data_box] = forms.IntegerField(required=False, initial=0)


Why not BooleanField, too?

Naltharial · 2012-10-27T15:26:28Z

Fixed.

mitar · 2012-10-29T09:14:07Z

piplmesh/account/forms.py

+ attrs[display_box] = forms.BooleanField(label=data_box, required=False)
+
+ # Connect display checkbox to data field
+ attrs[display_box].widget = forms.CheckboxInput(attrs={'data-panel':data_box, 'data-display':'True'})


Spaces after :.

Why are you settings widget from outside and not through the constructor? Set it through the constructor, it is much cleaner (and it might be better handled, you never know if there is some special handling when passed through constructor which you are bypassing now).

mitar · 2012-10-29T09:38:30Z

You had some problems with link property? Or you just didn't upgrade from requirements.txt?

mitar · 2012-11-06T08:16:40Z

I moved account Django app to separate Python package. You will have to update this pull request.

https://github.com/mitar/django-mongo-auth

mitar · 2012-11-10T19:49:59Z

OK, now everything should work with external authentication package.

Primoz Jeras added 3 commits September 10, 2012 11:47

Basic start for issue wlanslovenija#265 - added form to enable/disabl…

1589264

…e homepage panels.

Upstream merge.

b72a369

Panel ordering.

053e7ff

mitar reviewed Sep 12, 2012
View reviewed changes

Remade form model with metaclass, changed Panels model more similiar …

4b07a40

…to Issue wlanslovenija#278.

Primoz Jeras added 2 commits September 13, 2012 15:46

Remove debugging.

c9bcc0f

Merge remote-tracking branch 'upstream/master'

9ca619a

Primoz Jeras added 2 commits September 16, 2012 02:52

Improved consistency with Issue wlanslovenija#278.

5acfcbd

Merge remote-tracking branch 'upstream/master'

f26f03c

Added number_of_columns support to /panels/collapse/

ffcc520

mitar reviewed Sep 19, 2012
View reviewed changes

piplmesh/account/forms.py

"""

Form for selecting homepage panels.

"""

__metaclass__ = PanelFormMetaclass

Copy link

Member

mitar Sep 19, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty line after docstring.

mitar reviewed Oct 15, 2012
View reviewed changes

Primoz Jeras added 3 commits October 22, 2012 12:15

Ghost panel inputs.

b8352d1

Merge remote-tracking branch 'upstream/master'

39053de

Unneccesary.

87213c6

mitar reviewed Oct 22, 2012
View reviewed changes

Primoz Jeras added 2 commits October 27, 2012 17:23

BooleanField, improved input detection.

659b606

No debug.

444243d

Primoz Jeras added 2 commits October 27, 2012 17:27

No debug.

9b257c9

Revert.

6ae8f90

mitar reviewed Oct 29, 2012
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue #265 - user panel control #288

Issue #265 - user panel control #288

Naltharial commented Sep 10, 2012

mitar Sep 12, 2012

Naltharial commented Sep 13, 2012

mitar commented Sep 14, 2012

mitar commented Sep 14, 2012

Naltharial commented Sep 16, 2012

mitar commented Sep 16, 2012

Naltharial commented Sep 16, 2012

mitar commented Sep 16, 2012

Naltharial commented Sep 16, 2012

mitar Sep 19, 2012

Naltharial commented Oct 14, 2012

mitar Oct 15, 2012

mitar commented Oct 15, 2012

mitar commented Oct 15, 2012

Naltharial commented Oct 22, 2012

mitar Oct 22, 2012

Naltharial commented Oct 27, 2012

mitar Oct 29, 2012

mitar Oct 29, 2012

mitar commented Oct 29, 2012

mitar commented Nov 6, 2012

mitar commented Nov 10, 2012

Issue #265 - user panel control #288

Are you sure you want to change the base?

Issue #265 - user panel control #288

Conversation

Naltharial commented Sep 10, 2012

mitar Sep 12, 2012

Choose a reason for hiding this comment

Naltharial commented Sep 13, 2012

mitar commented Sep 14, 2012

mitar commented Sep 14, 2012

Naltharial commented Sep 16, 2012

mitar commented Sep 16, 2012

Naltharial commented Sep 16, 2012

mitar commented Sep 16, 2012

Naltharial commented Sep 16, 2012

mitar Sep 19, 2012

Choose a reason for hiding this comment

Naltharial commented Oct 14, 2012

mitar Oct 15, 2012

Choose a reason for hiding this comment

mitar commented Oct 15, 2012

mitar commented Oct 15, 2012

Naltharial commented Oct 22, 2012

mitar Oct 22, 2012

Choose a reason for hiding this comment

Naltharial commented Oct 27, 2012

mitar Oct 29, 2012

Choose a reason for hiding this comment

mitar Oct 29, 2012

Choose a reason for hiding this comment

mitar commented Oct 29, 2012

mitar commented Nov 6, 2012

mitar commented Nov 10, 2012