Frontend toolchain setup and some basic map widgets by chrisdevereux · Pull Request #4 · commonknowledge/groundwork

chrisdevereux · 2021-11-23T15:20:30Z

Description

This PR sets up the frontend build system and adds the first feature. There's quite a lot here – happy to walk through it.

In a little more detail, we add the following:

Frontend build system setup using vite. This is used to build the bundled js – doesn't impose any constraints on the build system used by the application.
Sets up jest and some helpers for unit testing stimulus controllers
Adds a helper to setup the stimulus app and load controllers on-demand (lazily, when first referenced on a page).
Adds, documents and tests stimulus controllers for:
- Rendering a mapbox map widget
- Adding datasources and layers 'over the wire' in html.
- A more general framework for hooking into the mapbox widget via additional stimulus controllers.
Adds, documents and tests django template tags for configuring and rendering a map widget without having to know about stimulus syntax.
Adds an example app that demonstrates the map widget.
Configures the CI to lint and test frontend code.

Although the APIs are all documented, we're missing a more high-level architectural overview, with information about how to use and extend this setup. I'm happy to add this onto this PR if wanted, but haven't yet as I'd like a bit of feedback on what you'd like explained.

How Can It Be Tested?

Follow the dev setup instructions.
Switch to this branch, run yarn and poetry install to pull in dependencies.
Run the 'Run example app' task in vscode.
Visit http://localhost:8000/geo/map/ and confirm that a map is rendered with a data-driven layer rendered on top.

Screenshots (if appropriate):

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation change

Checklist:

I have documented all new methods using google docstring format.
I have added type annotations to any public API methods.
I have updated any relevant high-level documentation.
I have added a usage example to the example app if relevant.
I have written tests covering all new changes.

conatus

I think this is cool. If we could put all dependencies in the package manager, this could be rolled out.

I'm a bit in two minds about using Vite. The more well known bundler is obviously Webpack. When building a library one wants to consider such things. However, if a Webpack build breaks I am often at a complete loss how to fix it. Probably more of a loss than some new technology I don't know. So going to roll the dice on this one, in the understanding that at some point we may want to switch it around.

I don't think anyone will come to this library and think "no, not for me, doesn't use Webpack" and turn elsewhere. Feel free to respond on this PR world if I am wrong!

conatus · 2021-11-24T15:48:47Z

example/templates/base.html

+
+    <script
+      type="module"
+      src="https://cdn.skypack.dev/@hotwired/turbo"


Is it better to use the bundler for these libraries now we have the bundler?

To be clear, this is for the example/desk-testing app, so has no impact on the library itself. Although, agree – we should bundle it so that you can desk-test without an internet connection.

conatus · 2021-11-24T15:50:18Z

example/templates/base.html

+    </script>
+    {% vite_hmr_client %} {% vite_asset 'example/frontend/main.ts' %}
+
+    <link


Though I do think Bootstrap should be in the mix here, to keep the commit history clean, I really think that it makes sense to not include it in this commit. What do you think?

This is just for the example app. Mostly just so we have access to the util classes and components when demonstrating the features. We're not adding any kind of dependency to the library (at this point – not sure whether we do or not longer term)

Understood. Go with what you think is best for desk testing this then loop me back in to give it the digital nod.

conatus · 2021-11-24T15:51:04Z

package.json

+  "dependencies": {},
+  "devDependencies": {
+    "@hotwired/stimulus": "^3.0.1",
+    "@hotwired/turbo": "^7.0.1",


So this is included in the template and in the bundler. Could we include it in one or the other? Seems only right to do so.

chrisdevereux · 2021-11-24T16:23:39Z

I think this is cool. If we could put all dependencies in the package manager, this could be rolled out.

We actually don't have any dependencies. Everything listed in devDependencies is one of:

A peer dependency (and listed in that section of package.json)
Build tooling
In the case of smaller dependencies, built into the library's js bundle.

I've gone for the last one because it reduces the api surface area (/scope for dependency hell) and has a minimal effect on the built bundle size. Open to reconsidering that approach though. I know people have differing opinions on this.

I'm a bit in two minds about using Vite. The more well known bundler is obviously Webpack. When building a library one wants to consider such things.

If it helps, Vite's production build mode wraps rollup.js, which is generally preferred over webpack for libraries.

(although the main reason I'm using it here is because it's so much quicker in dev mode)

janbaykara

I think Vite is a good choice to be honest! And in any case, as this is a library, bundling for userland will presumably result in plain file assets for hoovering up by Django's collectstatic command anyway. So the debate over bundler here is mainly for package developers which is a small crowd. Don't see too much controversy.

It'd be nice if MAPBOX_PUBLIC_API_KEY and MAPBOX_DEFAULT_STYLE were documented, for end users to use! Perhaps these could be auto-bundled with local.py, or an "env variables" section of the docs added.

janbaykara · 2021-11-25T16:35:06Z

pyck/geo/examples.py

+from typing import Any, List
+
+from django.urls import path
+from django.views.generic import TemplateView
+
+
+class MapExampleView(TemplateView):
+    template_name = "pyck/geo/examples/map_example.html"
+
+    @property
+    def sources(self):
+        return {
+            "mapbox-terrain": {
+                "type": "vector",
+                "url": "mapbox://mapbox.mapbox-terrain-v2",
+            }
+        }
+
+    @property
+    def layers(self):
+        return [
+            {
+                "id": "terrain-data",
+                "type": "line",
+                "source": "mapbox-terrain",
+                "source-layer": "contour",
+                "layout": {"line-join": "round", "line-cap": "round"},
+                "paint": {"line-color": "#ff69b4", "line-width": 1},
+            }
+        ]
+
+
+urlpatterns: List[Any] = [path("map/", MapExampleView.as_view())]


In a future release, we should try to include more domain-specific examples. For mapping, we could do something like a doorknocking app showing contacted addresses or somesuch.

janbaykara · 2021-11-25T16:44:41Z

pyck/geo/templates/pyck/geo/components/map_config.html

+<slot
+  data-controller="{{controller}}"
+  data-map-target="config"
+  {% for key, value in values.items %}
+  {% if value %}data-{{controller}}-{{key}}-value="{{value}}"{% endif %}
+  {% endfor %}
+  {% for key, value in attributes.items %}
+  {% if value %}{{key}}="{{value}}"{% endif %}
+  {% endfor %}
+>
+</slot>


No version of Internet Explorer supports the slot tag, but presumably it will will break the page rendering?

janbaykara · 2021-11-25T17:02:35Z

pyck/geo/frontend/controllers/map-controller.ts

+  private apiKeyValue!: string;
+  private styleValue!: string;
+  private centerValue!: string;
+  private zoomValue!: number;
+
+  private canvasTarget!: HTMLElement;
+  private configTargets!: HTMLElement[];


It's annoying that the Stimulus library can't somehow monkeypatch the types onto the class automatically!

Chris Devereux added 15 commits November 23, 2021 12:41

minor tweak to linting

1d8f0bb

adds frontend build pipeline

466122e

adds prettier and runs on all js source

3580e47

configure frontend build and tests

0a0d5db

setup test fixtures

350fd66

adds frontend implementation of map component

d4fb22f

move decorator typings out into utility module

d9a9c52

adds launch task for debugging django tests

49fce3c

adds (and tests) template tag for rendering a map widget

fb1cbc3

adds example app and usage example of the map widget

82fe635

fixes to code style and documentation

98e31b3

add manage.py

40682ed

adds support for adding sources and layers to the map

c35102d

document our map tags

e13ead0

typecheck in ci

b7d04cb

chrisdevereux requested review from conatus and janbaykara November 23, 2021 15:20

run yarn on bootstrap

de3f250

conatus suggested changes Nov 24, 2021

View reviewed changes

chrisdevereux merged commit 43fc2b3 into main Nov 24, 2021

chrisdevereux deleted the frontend-setup branch November 24, 2021 16:27

janbaykara reviewed Nov 25, 2021

View reviewed changes

Uh oh!

Comments

Conversation

chrisdevereux commented Nov 23, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Can It Be Tested?

Screenshots (if appropriate):

Types of changes

Checklist:

Uh oh!

conatus left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

conatus Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisdevereux commented Nov 24, 2021

Uh oh!

janbaykara left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chrisdevereux commented Nov 23, 2021 •

edited

Loading

conatus left a comment •

edited

Loading

conatus Nov 24, 2021 •

edited

Loading