Merge pull request #124 from bcdickinson/fix-base-template-wrapping

Fix broken page/fragment detection

Author: Ben Dickinson

.github/PULL_REQUEST_TEMPLATE.md

@@ -13,3 +13,4 @@ Fixes # (issue)
 - [ ] My changes generate no new warnings
 - [ ] I have added tests that prove my fix is effective or that my feature works
 - [ ] New and existing unit tests pass locally with my changes
+- [ ] I have added an appropriate CHANGELOG entry

CHANGELOG.md

@@ -13,6 +13,7 @@
 
 ### Fixed
 
+- Pages and fragments are now handled correctly again ([#119](https://github.com/torchbox/django-pattern-library/issues/119))
 - PyPI package metadata now uses absolute URLs to GitHub ([#120](https://github.com/torchbox/django-pattern-library/issues/120)).
 
 ## [0.2.9] - 2020-07-29
@@ -57,3 +58,8 @@
 
 ### Added
 - Compatibility with Django 2.2
+
+[0.2.9]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.9
+[0.2.8]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.8
+[0.2.5]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.5
+[0.2.4]: https://github.com/torchbox/django-pattern-library/releases/tag/v0.2.4

README.md

@@ -2,14 +2,13 @@
 
 [![PyPI](https://img.shields.io/pypi/v/django-pattern-library.svg)](https://pypi.org/project/django-pattern-library/) [![PyPI downloads](https://img.shields.io/pypi/dm/django-pattern-library.svg)](https://pypi.org/project/django-pattern-library/) [![Travis](https://travis-ci.com/torchbox/django-pattern-library.svg?branch=master)](https://travis-ci.com/torchbox/django-pattern-library) [![Total alerts](https://img.shields.io/lgtm/alerts/g/torchbox/django-pattern-library.svg?logo=lgtm&logoWidth=18)](https://lgtm.com/projects/g/torchbox/django-pattern-library/alerts/)
 
-A module for Django that helps you to build pattern libraries and follow the
-[Atomic design](http://bradfrost.com/blog/post/atomic-web-design/) methodology.
+A module for Django that helps you to build pattern libraries.
 
 ![Screenshot of the pattern library UI, with navigation, pattern rendering, and configuration](https://raw.githubusercontent.com/torchbox/django-pattern-library/master/.github/pattern-library-screenshot.webp)
 
 ## Documentation
 
-Documentation is located in GitHub in [`docs/`](https://github.com/torchbox/django-pattern-library/tree/master/docs).
+Documentation is located on GitHub in [`docs/`](https://github.com/torchbox/django-pattern-library/tree/master/docs).
 
 ## Objective
 
@@ -29,9 +28,38 @@ To learn more about how this package can be used, have a look at our Wagtail Spa
 
 [![Reusable UI components: A journey from React to Wagtail](https://raw.githubusercontent.com/torchbox/django-pattern-library/master/.github/pattern-library-talk-youtube.webp)](https://www.youtube.com/watch?v=isrOufI7TKc)
 
+## Concepts
+To understand how `django-pattern-library` works, the following concepts are important.
+
+### Patterns
+Any template that is displayed by the pattern library is referred to as a pattern. Patterns are divided into two categories: fragments and pages.
+
+### Fragments
+A fragment is a pattern whose markup does not include all of the resources (typically CSS and Javascript) for it to be displayed correctly on its own. This is typical for reusable component templates which depend on global stylesheets or Javascript bundles to render and behave correctly.
+
+To enable them to be correctly displayed in the pattern library, `django-pattern-library` will inject the rendered markup of fragments into the **pattern base template** specified by `PATTERN_LIBRARY['PATTERN_BASE_TEMPLATE_NAME']`.
+
+This template should include references to any required static files. The rendered markup of fragments will be available in the `pattern_library_rendered_pattern` context variable (see the tests for [an example](https://github.com/torchbox/django-pattern-library/blob/master/tests/templates/patterns/base.html)).
+
+### Pages
+In contrast to fragments, pages are patterns that include everything they need to be displayed correctly in their markup. Pages are defined by `PATTERN_LIBRARY['BASE_TEMPLATE_NAMES']`. 
+
+Any template in that list — or that extends a template in that list — is considered a page and will be displayed as-is when rendered in the pattern library.
+
+It is common practice for page templates to extend the pattern base template to avoid duplicate references to stylesheets and Javascript bundles. Again, [an example](https://github.com/torchbox/django-pattern-library/blob/master/tests/templates/patterns/base_page.html) of this can be seen in the tests.
+
 ## How to install
 
-In your Django settings, add `pattern_library` into your `INSTALLED_APPS`, and `pattern_library.loader_tags` into the `TEMPLATES` setting. For example:
+First install the library:
+
+```sh
+pip install django-pattern-library
+# ... or...
+poetry add django-pattern-library
+```
+
+
+Then, in your Django settings, add `pattern_library` into your `INSTALLED_APPS`, and `pattern_library.loader_tags` to `OPTIONS['builtins']` into the `TEMPLATES` setting. For example:
 
 ```python
 INSTALLED_APPS = [
@@ -58,18 +86,42 @@ TEMPLATES = [
 ]
 ```
 
-Note that this module only supports the Django template backend out of the box.
+Note that this module only supports the Django template backend.
 
-Set the `PATTERN_LIBRARY_TEMPLATE_DIR` setting to point to a template directory with your patterns:
+### Settings
+
+Next, set the `PATTERN_LIBRARY` setting. Here's an example showing the defaults:
 
 ```python
-PATTERN_LIBRARY_TEMPLATE_DIR = os.path.join(BASE_DIR, 'project_styleguide', 'templates')
+PATTERN_LIBRARY = {
+    # PATTERN_BASE_TEMPLATE_NAME is the template that fragments will be wrapped with.
+    # It should include any required CSS and JS and output
+    # `pattern_library_rendered_pattern` from context.
+    'PATTERN_BASE_TEMPLATE_NAME': 'patterns/base.html',
+    # Any template in BASE_TEMPLATE_NAMES or any template that extends a template in
+    # BASE_TEMPLATE_NAMES is a "page" and will be rendered as-is without being wrapped.
+    'BASE_TEMPLATE_NAMES': ['patterns/base_page.html'],
+    'TEMPLATE_SUFFIX': '.html',
+    # SECTIONS controls the groups of templates that appear in the navigation. The keys
+    # are the group titles and the values are lists of template name prefixes that will
+    # be searched to populate the groups.
+    'SECTIONS': (
+        ('atoms', ['patterns/atoms']),
+        ('molecules', ['patterns/molecules']),
+        ('organisms', ['patterns/organisms']),
+        ('templates', ['patterns/templates']),
+        ('pages', ['patterns/pages']),
+    ),
+}
+
 ```
 
-Note that `PATTERN_LIBRARY_TEMPLATE_DIR` must be available for
-[template loaders](https://docs.djangoproject.com/en/1.11/ref/templates/api/#loader-types).
+Note that the templates in your `PATTERN_LIBRARY` settings must be available to your project's
+[template loaders](https://docs.djangoproject.com/en/3.1/ref/templates/api/#loader-types).
+
+### URLs
 
-Include `pattern_library.urls` into your `urlpatterns`. Here's an example `urls.py`:
+Include `pattern_library.urls` in your `urlpatterns`. Here's an example `urls.py`:
 
 ```python
 from django.apps import apps

pattern_library/__init__.py

@@ -1,10 +1,17 @@
-
-
 default_app_config = 'pattern_library.apps.PatternLibraryAppConfig'
 
 DEFAULT_SETTINGS = {
-    'BASE_TEMPLATE_NAME': 'patterns/base.html',
+    # PATTERN_BASE_TEMPLATE_NAME is the template that fragments will be wrapped with.
+    # It should include any required CSS and JS and output
+    # `pattern_library_rendered_pattern` from context.
+    'PATTERN_BASE_TEMPLATE_NAME': 'patterns/base.html',
+    # Any template in BASE_TEMPLATE_NAMES or any template that extends a template in
+    # BASE_TEMPLATE_NAMES is a "page" and will be rendered as-is without being wrapped.
+    'BASE_TEMPLATE_NAMES': ['patterns/base_page.html'],
     'TEMPLATE_SUFFIX': '.html',
+    # SECTIONS controls the groups of templates that appear in the navigation. The keys
+    # are the group titles and the value are lists of template name prefixes that will
+    # be searched to populate the groups.
     'SECTIONS': (
         ('atoms', ['patterns/atoms']),
         ('molecules', ['patterns/molecules']),
@@ -15,25 +22,27 @@
 }
 
 
-def get_from_settings(attr):
+def get_setting(attr):
     from django.conf import settings
-
     library_settings = DEFAULT_SETTINGS.copy()
     library_settings.update(getattr(settings, 'PATTERN_LIBRARY', {}))
-
     return library_settings.get(attr)
 
 
 def get_pattern_template_suffix():
-    return get_from_settings('TEMPLATE_SUFFIX')
+    return get_setting('TEMPLATE_SUFFIX')
 
 
 def get_pattern_base_template_name():
-    return get_from_settings('BASE_TEMPLATE_NAME')
+    return get_setting('PATTERN_BASE_TEMPLATE_NAME')
+
+
+def get_base_template_names():
+    return get_setting('BASE_TEMPLATE_NAMES')
 
 
 def get_sections():
-    return get_from_settings('SECTIONS')
+    return get_setting('SECTIONS')
 
 
 def get_pattern_context_var_name():

pattern_library/loader_tags.py

@@ -17,7 +17,8 @@ class ExtendsNode(DjangoExtendsNode):
     """
     def render(self, context):
         if is_pattern_library_context(context):
-            parent_context = get_pattern_context(self.parent_name.var)
+            parent_name = self.parent_name.resolve(context)
+            parent_context = get_pattern_context(parent_name)
             if parent_context:
                 # We want parent_context to appear later in the lookup process
                 # than context of the actual template.
@@ -59,7 +60,8 @@ class IncludeNode(DjangoIncludeNode):
     """
     def render(self, context):
         if is_pattern_library_context(context):
-            pattern_context = get_pattern_context(self.template.var)
+            template = self.template.resolve(context)
+            pattern_context = get_pattern_context(template)
             extra_context = {name: var.resolve(context) for name, var in self.extra_context.items()}
 
             if self.isolated_context:
@@ -89,8 +91,8 @@ def render(self, context):
 @register.tag('extends')
 def do_extends(parser, token):
     """
-    Copy if Django's built-in {% extends ... %} tag that uses the custom
-    ExtendsNode to allow us to load dump data for pattern library.
+    A copy of Django's built-in {% extends ... %} tag that uses our custom
+    ExtendsNode to allow us to load dummy context for the pattern library.
     """
     bits = token.split_contents()
     if len(bits) != 2:
@@ -106,8 +108,8 @@ def do_extends(parser, token):
 @register.tag('include')
 def do_include(parser, token):
     """
-    Copy if Django's built-in {% include ... %} tag that uses the custom
-    IncludeNode to allow us to load dump data for pattern library.
+    A copy of Django's built-in {% include ... %} tag that uses our custom
+    IncludeNode to allow us to load dummy context for the pattern library.
     """
     bits = token.split_contents()
     if len(bits) < 2:

pattern_library/utils.py

@@ -3,7 +3,9 @@
 import re
 
 from django.template import TemplateDoesNotExist
+from django.template.context import Context
 from django.template.loader import get_template, render_to_string
+from django.template.loader_tags import ExtendsNode
 from django.template.loaders.app_directories import get_app_template_dirs
 from django.utils.safestring import mark_safe
 
@@ -189,3 +191,27 @@ def render_pattern(request, template_name, allow_non_patterns=False):
     context = get_pattern_context(template_name)
     context[get_pattern_context_var_name()] = True
     return render_to_string(template_name, request=request, context=context)
+
+
+def get_template_ancestors(template_name, context=None, ancestors=None):
+    """
+    Returns a list of template names, starting with provided name
+    and followed by the names of any templates that extends until
+    the most extended template is reached.
+    """
+    if ancestors is None:
+        ancestors = [template_name]
+
+    if context is None:
+        context = Context()
+
+    pattern_template = get_template(template_name)
+
+    for node in pattern_template.template.nodelist:
+        if isinstance(node, ExtendsNode):
+            parent_template_name = node.parent_name.resolve(context)
+            ancestors.append(parent_template_name)
+            get_template_ancestors(parent_template_name, context=context, ancestors=ancestors)
+            break
+
+    return ancestors

pattern_library/views.py

@@ -1,17 +1,20 @@
-from django.http import Http404
+from django.http import Http404, HttpResponse
 from django.template.loader import get_template
 from django.utils.decorators import method_decorator
 from django.utils.html import escape
 from django.views.decorators.clickjacking import xframe_options_sameorigin
 from django.views.generic.base import TemplateView
 
-from pattern_library import get_pattern_base_template_name
+from pattern_library import (
+    get_base_template_names, get_pattern_base_template_name
+)
 from pattern_library.exceptions import (
     PatternLibraryEmpty, TemplateIsNotPattern
 )
 from pattern_library.utils import (
-    get_pattern_config, get_pattern_config_str, get_pattern_markdown,
-    get_pattern_templates, get_sections, is_pattern, render_pattern
+    get_pattern_config, get_pattern_config_str, get_pattern_context,
+    get_pattern_markdown, get_pattern_templates, get_sections,
+    get_template_ancestors, is_pattern, render_pattern
 )
 
 
@@ -74,12 +77,20 @@ class RenderPatternView(TemplateView):
 
     @method_decorator(xframe_options_sameorigin)
     def get(self, request, pattern_template_name=None):
+        pattern_template_ancestors = get_template_ancestors(
+            pattern_template_name,
+            context=get_pattern_context(self.kwargs['pattern_template_name']),
+        )
+        pattern_is_fragment = set(pattern_template_ancestors).isdisjoint(set(get_base_template_names()))
+
         try:
             rendered_pattern = render_pattern(request, pattern_template_name)
         except TemplateIsNotPattern:
             raise Http404
 
-        context = self.get_context_data()
-        context['pattern_library_rendered_pattern'] = rendered_pattern
+        if pattern_is_fragment:
+            context = self.get_context_data()
+            context['pattern_library_rendered_pattern'] = rendered_pattern
+            return self.render_to_response(context)
 
-        return self.render_to_response(context)
+        return HttpResponse(rendered_pattern)

tests/settings.py

@@ -24,6 +24,7 @@
     'SECTIONS': [
         ('atoms', ['patterns/atoms']),
         ('molecules', ['patterns/molecules']),
+        ('pages', ['patterns/pages']),
     ],
 }
 

tests/templates/non-patterns/variable_include.html

@@ -0,0 +1 @@
+included content from variable

tests/templates/patterns/atoms/test_extends/base.html

@@ -0,0 +1 @@
+{% block content %}base content{% endblock %}

tests/templates/patterns/atoms/test_extends/extended.html

@@ -0,0 +1,3 @@
+{% extends parent_template_name %}
+
+{% block content %}{{ block.super }} - extended content{% endblock %}

tests/templates/patterns/atoms/test_extends/extended.yaml

@@ -0,0 +1,2 @@
+context:
+    parent_template_name: patterns/atoms/test_extends/base.html

tests/templates/patterns/atoms/test_includes/test_includes.html

@@ -1,5 +1,6 @@
 {% load test_tags %}
 
 {% include 'non-patterns/include.html' %}
+{% include variable_include %}
 
 {% error_tag include %}

tests/templates/patterns/atoms/test_includes/test_includes.yaml

@@ -1,3 +1,6 @@
+context:
+    variable_include: non-patterns/variable_include.html
+
 tags:
   error_tag:
     include:

tests/templates/patterns/base.html

@@ -1 +1,8 @@
-{% block content %}{{ pattern_library_rendered_pattern }}{% endblock %}
+<html lang="en-GB">
+    <head>
+        <title>{% block title %}Fragment{% endblock %}</title>
+    </head>
+    <body>
+        {% block content %}{{ pattern_library_rendered_pattern }}{% endblock %}
+    </body>
+</html>

tests/templates/patterns/base_page.html

@@ -0,0 +1,3 @@
+{% extends 'patterns/base.html' %}
+
+{% block title %}Page{% endblock %}

tests/templates/patterns/pages/test_page/test_page.html

@@ -0,0 +1,3 @@
+{% extends 'patterns/base_page.html' %}
+
+{% block content %}{{ page.body }}{% endblock %}

tests/templates/patterns/pages/test_page/test_page.yaml

@@ -0,0 +1,11 @@
+context:
+    page:
+        body: >
+            <p>
+                Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod
+                tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam,
+                quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo
+                consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse
+                cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat
+                non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
+            </p>