Closed tonysyu closed 6 years ago
Hi,
that's a great idea! Any contribution is be appreciated :)
Please, create a new module widgets.contrib
with your widgets and let me know if I you need any help.
Please, create a new module widgets.contrib with your widgets
Do you mean I should rename widgets.simple
to widgets.contrib
or are you suggesting a new contrib
sub-package such that the widgets are in widgets.contrib.simple
Putting it in widgets.contrib.simple
sounds good.
BTW, the test failures are due to changes in Django 2.1 (and not related to this PR). It seems that the dashboard used for testing use widgets that don't define template_name
. The call to get_template_name
from the dashboard template throws an error when Widget.template_name
isn't defined, but before Django 2.1 the error was suppressed. It seems that this error gets bubbled up in Django 2.1. (I don't see any mention of this in release notes, though)
Merging #24 into develop will not change coverage. The diff coverage is
100%
.
@@ Coverage Diff @@
## develop #24 +/- ##
======================================
Coverage 100% 100%
======================================
Files 8 9 +1
Lines 369 405 +36
======================================
+ Hits 369 405 +36
Impacted Files | Coverage Δ | |
---|---|---|
controlcenter/templatetags/controlcenter_tags.py | 100% <100%> (ø) |
:arrow_up: |
controlcenter/utils.py | 100% <100%> (ø) |
:arrow_up: |
controlcenter/widgets/contrib/simple.py | 100% <100%> (ø) |
|
controlcenter/widgets/charts.py | 100% <0%> (ø) |
:arrow_up: |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update da9ff4b...acfc007. Read the comment docs.
@byashimov: I've moved the widgets to widgets.contrib.simple
, added tests, and added examples to the docs. In addition, I added a sample project, which adds quite a bit to the diff. If you'd prefer to take it out, I can easily just revert the last commit.
I don't really understand the results from @codecov-io: When I run the following locally, the new widgets and template tag have 100% coverage:
$ coverage run test_project/manage.py test; coverage report -m
Hi! It's fixed in develop now. Could you rebase your branch, please?
Oh, the example project is really awesome and you have done a great job, but there is already the test_project and docs with verbose examples. And I'm not really sure if I will have enough time to make compatibility fixes in future for it too. So if you don't mind I'll accept the feature only. Thanks.
Rebased and tests are passing! Also removed the example project.
@byashimov: Anything else?
Hey! Sorry for a long time review, just came back from my vacation. I've realised there is a lot I would rewrite in your PR after a closer look. If you don't mind I would do a few commits to it.
Marking this as a WIP since this is currently missing examples, documentation, and tests. I'd like to check that this would be a welcome addition before putting in the effort to finish this out.
Basically, this adds a couple of widgets,
ValueList
andKeyValueList
which can be used to render dashboard widgets from python lists and dictionaries instead of Django models. For example: