fix: stop hijacking COTTON_DIR — cotton/cf/ template layout#4
Merged
fix: stop hijacking COTTON_DIR — cotton/cf/ template layout#4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cf-ui0.1.0 is broken for any Django consumer that ships its owntemplates/cotton/<their-app>/...tree. The previous fix(5814476, "set COTTON_DIR (singular) in CfUiConfig") made
<c-cf.*>components resolvable by writing
settings.COTTON_DIR = "cotton/<theme>"in
CfUiConfig.ready(). But django-cotton usesCOTTON_DIRas a singleglobal prefix for every
<c-foo.bar>lookup, so the moment aconsumer added
cf_ui.django.CfUiConfigtoINSTALLED_APPS, every oneof its own components started raising
TemplateDoesNotExist: cotton/bulma/<their-path>.This PR adopts Option A from the issue brief: move cf-ui's cotton
templates to a path that doesn't require a global prefix.
What changed
src/cf_ui/templates/cotton/bulma/cf/*.html→src/cf_ui/templates/cotton/cf/*.html(14 components, all
git mv— no content edits).CfUiConfig.ready()is now a no-op. Django'sAPP_DIRS=Truewalkalready exposes the package templates; the django-cotton default
COTTON_DIR="cotton"resolves<c-cf.foo>tocotton/cf/foo.htmlcorrectly.
cf_ui.djangomodule just declares the AppConfig.Why Option A over Option B
A custom
app_directories.Loadersubclass that scopes the theme prefixto the
cotton/cf/subtree (Option B) would have preserved the existingfile layout, but at the cost of:
CfUiConfig,to rewrite it,
hides the path from
TemplateDoesNotExist).The README already states "Bulma — only supported theme" for v0.1, and
the
cotton/{bootstrap,daisy,fomantic,foundation}/directories arePLANNED.mdstubs. Multi-theme support, when it lands, will happeninside the templates (context-flag class names, or
{% include "cotton/cf/_themes/<theme>/<name>.html" %}partials) —the directory layout no longer has to encode it.
Consumer migration
Most consumers: nothing to do. The bug only surfaced once they added
cf_ui.django.CfUiConfigtoINSTALLED_APPS, and the workaround(setting
COTTON_DIR="cotton"explicitly to undo what cf-ui did) isno longer needed — they should just remove that line.
The escape hatch
from cf_ui import COTTON_TEMPLATES_DIRstill works;the relevant subdir changed from
COTTON_TEMPLATES_DIR / "bulma"toCOTTON_TEMPLATES_DIR / "cf". Documented in the README.Version
Bumped to 0.1.1. This is a bugfix (the package was broken for
the documented Django consumer use case), and the on-disk template
layout is not part of the public API — consumers use
<c-cf.*>/<CfCard>, which are unchanged. CHANGELOG entry added.Tests
tests/unit/test_consumer_compatibility.py(3 tests):CfUiConfig.ready()does not setCOTTON_DIR="cotton/bulma"— pinning the regression.Enginethat combines a tmpdir consumer templateat
cotton/myapp/foo.htmlwith cf-ui's app-templates, andasserts both
cotton/myapp/foo.htmlandcotton/cf/notification.htmlresolve through the same engine.This is the direct repro of the RankedJobs failure.
cotton/cf/<name>.html.tests/unit/test_core.py::test_django_appconfig_ready_sets_cotton_dir→
test_django_appconfig_does_not_override_cotton_dir.tests/unit/cotton/conftest.pytemplate prefix(
cotton/bulma/...→cotton/...).tests/e2e/_e2e_django_settings.py— removed the now-unnecessaryCOTTON_DIR = "cotton/bulma"override.uv run pytest tests/ -v(excluding the e2e suite, which needs aplaywright browser binary unavailable in this sandbox): 117 passed.
E2E settings module loads cleanly and
get_template("cotton/cf/notification.html")resolves end-to-end against
_e2e_django_settings.uv run ruff check src/ tests/anduv run ruff format src/ tests/both clean.
Test plan
cf-ui==0.1.1, confirm<c-layouts.base>and otherconsumer-defined cotton components resolve again, and
<c-cf.notification>/<c-cf.card>still render.COTTON_DIR="cotton"as a workaround can drop that override.https://claude.ai/code/session_01C8NrnRLghd26y2Qs325pvQ
Generated by Claude Code