-
-
-
-
Hm - I'm not sure if it makes the most sense to put the widgets in here. Maybe there should be a widgets.py within the admin subdirectory?
-
-
-
Hm - why the extraneous spaces around widget_data.x within the {'s and }'s? Some of these lines seem a bit long and unruly to me. Also, those strings need to be i18n'able. -
-
-
Feedback for Dashboard widget logic
Review Request #2361 — Created May 17, 2011 and discarded
I would like to get some feedback for this dashboard logic and code style formatting.
The dashboard.html template would have this:
Example: {% admin_widget "user-activity" _("User Activity") %}
I followed the same pattern as {% admin_subnav .... %}
'admin/admin_widget.html' - is a wrap for all widgets
'w-user-activity.html' - is a template that includes the necessary HTML and JavaScript for "User Activity" and is 'embedded' as '{% include widget_content %}' in 'admin_widget.html'
'rbadmintags.py' - admin_widget(...) has a list of all possible widgets in "widgets = {}".
Screenshots
| Description | From | Last Updated |
|---|---|---|
|
You're going to want to put these spaces back. We put two spaces between class-level items in code. |
 mike_conley
|
|
|
I think we'll want to be consistent with our naming, and call these media_url and media_serial. |
mike_conley
|
|
|
Hm - I'm not sure if it makes the most sense to put the widgets in here. Maybe there should … |
mike_conley
|
|
|
We're going to want this i18n'able. |
mike_conley
|
|
|
You'll probably want to put in a more descriptive comment here. ;) |
mike_conley
|
|
|
Hm - why the extraneous spaces around widget_data.x within the {'s and }'s? Some of these lines seem a bit … |
mike_conley
|
|
|
Same, re: more descriptive comment. |
mike_conley
|
|
|
Spaces on either side of the + |
mike_conley
|
|
|
"Total Users" needs to be wrapped up for i18n. |
mike_conley
|
|
|
Because we're starting to load from a different package, I think we put a newline between from django.core.urlresolvers... and from … |
mike_conley
|
|
|
Two spaces here. |
mike_conley
|
|
|
I'd like some documentation describing what this widget actually does. |
mike_conley
|
|
|
I have a feeling that some of these lines are still over the 80 char limit - can you break … |
mike_conley
|
|
|
{{widget_size}} as opposed to {{ widget_size }} - same with widget_name. |
mike_conley
|
|
|
See above. |
mike_conley
|
|
|
Any way of making "Active", "7 days ago", etc, i18n-able? |
mike_conley
|
|
|
Whoops - should have been more clear. I meant that there *should* be spaces on either side of the +'s. … |
mike_conley
|
|
|
I'm not sure this block needs to be indented. |
mike_conley
|
-
Vlad: It's looking good! A few more things caught my eye this iteration - see below. Thanks, -Mike
-
Because we're starting to load from a different package, I think we put a newline between from django.core.urlresolvers... and from reviewboard.admin...
-
-
-
I have a feeling that some of these lines are still over the 80 char limit - can you break them up a bit to shorten them?
-
-
-
-
Whoops - should have been more clear. I meant that there *should* be spaces on either side of the +'s. :)
-