-
Notifications
You must be signed in to change notification settings - Fork 1
Feat: UI templates for the districts
application
#98
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
99698c3
to
450f24a
Compare
districts
application
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
450f24a
to
8285883
Compare
|
8285883
to
46f0969
Compare
I've been thinking about this some as it relates to our A couple of ideas I'm kicking around: Hierarchical contextIt is OK (and maybe preferable for organization) to have hierarchical context values using def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
districts_ctx = {}
districts_ctx["all"] = District.objects.all()
context["districts"] = districts_ctx
return context Then in the template: <h1>Districts</h1>
{% for district in districts.all %}
...
{% endfor %} Standard contextWe might want to think about standardizing on some of the top-level keys. E.g. if we have a class-based view that adds context via a class SomeView(TemplateView):
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
custom_ctx = {}
custom_ctx["something"] = "data"
context["view"] = custom_ctx
return context This way we can easily distinguish between that from views and that from context processors in the template: <h1>{{ view.title }}</h1>
<p>{{ view.content }}</p>
<p>{{ common.content }}</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left some comments.
I think it's time to look into more of the class-based views. A lot of the work here would come for free by using some more sophisticated views beyond TemplateView
.
context that will be shared across views of the districts app is moved into the DistrictContextMixin. This reduces context duplication, both when different views need the same context and also when the view may not need the context, but the template it extends does need it. Note that because of how the MRO and mixins work, DistrictContextMixin inherits from TemplateView.
using DetailView for the DistrictView class simplifies the code and improves readability in the districts/district.html template.
Thanks for the suggestions @thekaveman! |
now the URL parameter is more descriptive, it is the district number, and the template context refers to the currently selected district
806efc8
to
6f2679c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Closes #68
This PR adds UI placeholders for the
districts
application.Reviewing
Click around to test out the navigation sidebar to go from all districts to a particular district.