-
Notifications
You must be signed in to change notification settings - Fork 2
feat: GH-999 let CMS admin change header logo #1083
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
base: main
Are you sure you want to change the base?
Changes from 12 commits
e2f7c53
f52b8b2
ead8569
53cfef6
e1d19d9
9fef1c3
7459528
d156af6
d5b2a36
c7f4454
15a1c2d
053fdd7
79f14b3
c42f211
15fc05c
07c0c2a
27fa19d
96ed5cd
7070552
3ee0438
fc6bb82
1937592
fc7eb38
0d4fd91
42a801a
c85cc71
2cabc1c
e56e434
b737a87
34988a7
440ceba
606a0eb
559dbf7
9beaf61
a4f1a43
1d01d08
9ff3bc2
958e340
74241ec
56931f1
f878b60
97d3382
d8d6114
7450f3f
a6b9209
4c79ddf
cead748
950ed99
d361e90
1d96a54
39a8386
b41b1a6
791d412
0d31361
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # GH-999: Editable header (`header-content`) | ||
|
|
||
| Plan for [#999 Let CMS Admin Edit Header](https://github.com/TACC/Core-CMS/issues/999) and stacked work on [`feat/GH-999-let-cms-admin-edit-header`](https://github.com/TACC/Core-CMS/pull/1083). | ||
|
|
||
| ## Goals | ||
|
|
||
| | Priority | Item | This stack | | ||
| | --- | --- | --- | | ||
| | 1 | CMS-editable logo | [#1083](https://github.com/TACC/Core-CMS/pull/1083) (current) | | ||
| | 2 | Nav collapse breakpoint | Follow-up issue (not this stack) | | ||
| | 3 | Custom branding | PR 3 (TACC Header Branding plugin) | | ||
| | 4–6 | Portal nav, search, branding order | Later thin plugins / options | | ||
|
|
||
| **Backwards compatibility:** If `header-content` has no plugins, render the header exactly as today (settings-driven branding + logo + nav). | ||
|
|
||
| **Precedence:** CMS plugin wins when present; otherwise settings (`LOGO` / `PORTAL_LOGO`, `BRANDING` / `PORTAL_BRANDING`). | ||
|
|
||
| ## Portal logo (HeaderLogoPlugin) | ||
|
|
||
| - **One plugin:** [`HeaderLogoPlugin`](taccsite_cms/contrib/taccsite_header_logo/cms_plugins.py) extends Picture (same model, no migration). Toolbar: **TACC Header → Header logo** — full Picture fields/templates; default `id="header-logo"` when Attributes omit `id`. | ||
| - **Render:** Standard Picture template at `{% static_placeholder "header-content" %}`. | ||
| - **Empty placeholder:** `or` fallback [`header_logo_via_settings.html`](taccsite_cms/templates/header_logo_via_settings.html) (`LOGO` / `PORTAL_LOGO`). | ||
| - **Branding images (PR 3):** nested Pictures under TACC Header Branding — not top-level. | ||
|
|
||
| ## PR stack | ||
|
|
||
| 1. **#1083 → `main`:** `header-content` static placeholder, superuser-only edit, settings fallback. | ||
| 2. **PR 1.75** (base: feature branch): `HeaderLogoPlugin` — correct markup + CMS edit UX (targets #1083 branch). | ||
| 3. **PR 2** (base: feature branch): Full-header orchestrator (`header_nav.html`, etc.). | ||
| 4. **PR 3** (base: after PR 2): TACC Header Branding plugin; add to `CMS_PLACEHOLDER_CONF`. | ||
|
|
||
| After #1083 merges, rebase the feature branch onto `main` before the stack lands on `main`. | ||
|
|
||
| ## Template files | ||
|
|
||
| Portal/Guide repos duplicate header markup ([Confluence](https://confluence.tacc.utexas.edu/x/LoCnCQ)). **`base.html` keeps `{% include "header.html" %}` only.** | ||
|
|
||
| | File | Role | | ||
| | --- | --- | | ||
| | [`header.html`](../taccsite_cms/templates/header.html) | Branding + nav; full `{% static_placeholder "header-content" or %}` + settings fallback. | | ||
| | [`header_logo.html`](../taccsite_cms/templates/header_logo.html) | Core-Portal `/cms/header/logo/markup/` — logo-only via [`render_header_logo`](../taccsite_cms/templatetags/header_tags.py) or settings. | | ||
| | [`header_logo_via_settings.html`](../taccsite_cms/templates/header_logo_via_settings.html) | Settings-only logo (`or` fallback). | | ||
| | [`render.py`](../taccsite_cms/contrib/taccsite_header_logo/render.py) | `render` — published logo plugin only (reuse in PR 2). | | ||
| | `header_nav.html` | **PR 2** — extract `<nav>…</nav>`. | | ||
|
|
||
| ## Testing | ||
|
|
||
| ### #1083 + PR 1.75 | ||
|
|
||
| 1. **Backwards compatibility:** Empty `header-content` → settings logo; branding/nav unchanged. | ||
| 2. **Superuser — logo:** Add **Header logo** plugin (image + optional link). Publish. Nav shows custom logo; placeholder still editable in `?edit`. | ||
| 3. **Superuser — footer:** `footer-content` unchanged. | ||
| 4. **Non-superuser:** Cannot edit static placeholders. | ||
| 5. **Limits:** One `HeaderLogoPlugin` in `header-content` (`global` 2 reserved for PR 3). | ||
| 6. **Core-Portal markup URL:** `/cms/header/logo/markup/` uses published `header-content` logo plugin or settings (not draft/edit toolbar). | ||
| 7. **Permissions after deploy:** Group perms; re-check step 4. | ||
|
|
||
| ### PR 2 (orchestrator) | ||
|
|
||
| 1. Empty placeholder → same as today. | ||
| 2. Header logo plugin → full nav + settings branding. | ||
| 3. Edit UX for `header-content` at header level. | ||
|
|
||
| ### PR 3 (branding plugin) | ||
|
|
||
| 1. Branding plugin + header logo → both slots; settings branding hidden when plugin present. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| from cms.plugin_pool import plugin_pool | ||
| from django.utils.translation import gettext_lazy as _ | ||
|
|
||
| from djangocms_picture.cms_plugins import PicturePlugin | ||
| from djangocms_picture.models import Picture | ||
|
|
||
| HEADER_LOGO_ELEMENT_ID = 'header-logo' | ||
|
|
||
|
|
||
| @plugin_pool.register_plugin | ||
| class HeaderLogoPlugin(PicturePlugin): | ||
| """ | ||
| Header > "Header logo" plugin | ||
|
|
||
| Full Picture plugin; default id="header-logo" when not set in Attributes. | ||
| """ | ||
| model = Picture | ||
| module = _('TACC Header') | ||
| name = _('Header logo') | ||
|
|
||
| def render(self, context, instance, placeholder): | ||
| if not instance.attributes: | ||
| instance.attributes = {} | ||
| if 'id' not in instance.attributes: | ||
| instance.attributes['id'] = HEADER_LOGO_ELEMENT_ID | ||
| return super().render(context, instance, placeholder) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| from cms.models import StaticPlaceholder | ||
| from cms.plugin_rendering import ContentRenderer | ||
|
|
||
| from taccsite_cms.contrib.taccsite_header_logo.cms_plugins import HeaderLogoPlugin | ||
|
|
||
|
|
||
| def render(request, context): | ||
| """ | ||
| Render published HeaderLogoPlugin from header-content (logo-only fragment). | ||
| """ | ||
| static_ph = StaticPlaceholder.objects.filter(code='header-content').first() | ||
| if not static_ph: | ||
| return '' | ||
|
|
||
| placeholder = static_ph.public | ||
| placeholder.is_static = True | ||
|
|
||
| renderer = ContentRenderer(request) | ||
| language = renderer.request_language | ||
|
|
||
| for plugin in renderer.get_plugins_to_render(placeholder, language, template=None): | ||
| if plugin.plugin_type == HeaderLogoPlugin.__name__: | ||
| return renderer.render_plugin( | ||
| plugin, | ||
| context, | ||
| placeholder, | ||
| editable=False, | ||
| ) | ||
| return '' |
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -499,7 +499,8 @@ def gettext(s): return s | |
| 'taccsite_cms.contrib.taccsite_sample', | ||
| 'taccsite_cms.contrib.taccsite_offset', | ||
| 'taccsite_cms.contrib.taccsite_system_specs', | ||
| 'taccsite_cms.contrib.taccsite_data_list' | ||
| 'taccsite_cms.contrib.taccsite_data_list', | ||
| 'taccsite_cms.contrib.taccsite_header_logo', | ||
| ] | ||
|
|
||
| # Convert list of paths to list of dotted module names | ||
|
|
@@ -564,7 +565,16 @@ def get_subdirs_as_module_names(path): | |
| } | ||
|
|
||
| CMS_PERMISSION = True | ||
| CMS_PLACEHOLDER_CONF = {} | ||
| CMS_PLACEHOLDER_CONF = { | ||
| 'header-content': { | ||
| # HeaderLogoPlugin (PR 1.75); room for TaccHeaderBrandingPlugin (PR 3) | ||
|
wesleyboar marked this conversation as resolved.
Outdated
|
||
| 'plugins': ['HeaderLogoPlugin'], | ||
| 'limits': { | ||
| 'global': 2, | ||
| 'HeaderLogoPlugin': 1, | ||
| }, | ||
| }, | ||
|
Comment on lines
+571
to
+581
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Name the placeholder and limit its content to a header logo. |
||
| } | ||
|
|
||
| THUMBNAIL_HIGH_RESOLUTION = True | ||
| THUMBNAIL_PROCESSORS = ( | ||
|
|
@@ -673,7 +683,7 @@ def get_subdirs_as_module_names(path): | |
| if 'BRANDING' not in locals(): | ||
| BRANDING = False | ||
|
|
||
| # For header_logo.html | ||
| # For header_logo_via_settings.html (logo slot fallback) | ||
|
wesleyboar marked this conversation as resolved.
Outdated
|
||
| deprecated_SETTINGS_EXPORT += ['LOGO'] | ||
| if 'LOGO' not in locals(): | ||
| LOGO = False | ||
|
|
||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The keystone code is in this file. |
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not as big a change a it looks. I basically moved it to what file looks like, sans diff |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,41 +1,7 @@ | ||
| {# @var logo, className #} | ||
| {% load static custom_portal_settings %} | ||
|
|
||
| {% if settings.LOGO %} | ||
|
|
||
| {% with settings.LOGO as logo %} | ||
| {% with filename=logo|index:1 selectors=logo|index:2 targeturl=logo|index:3 targettype=logo|index:4 accessibility=logo|index:5 corstype=logo|index:6 visibility=logo|index:7 %} | ||
| {% if visibility == "True" %} | ||
| <a id="header-logo" class="navbar-brand {{className}}" href="{{ targeturl }}" target="{{ targettype }}"> | ||
| <img class="portal-logo {{ selectors }}" src="{% static filename %}" crossorigin="{{ corstype }}" alt="{{ accessibility }}" /> | ||
| </a> | ||
| {% endif %} | ||
| {% endwith %} | ||
| {% endwith %} | ||
|
|
||
| {% load header_tags %} | ||
| {% render_header_logo as header_logo %} | ||
| {% if header_logo %} | ||
| {{ header_logo }} | ||
| {% else %} | ||
|
|
||
| {% with settings.PORTAL_LOGO as logo %} | ||
| <a | ||
| id="header-logo" | ||
| class="navbar-brand {{className}}" | ||
| href="{{ logo.link_href }}" | ||
| target="{{ logo.link_target }}" | ||
| {% if logo.link_name %} | ||
| aria-label="{{ logo.link_name }}" | ||
| {% endif %} | ||
| > | ||
| <img | ||
| class="portal-logo {{ logo.img_class }}" | ||
| src="{% if logo.is_remote %}{{ logo.img_file_src }}{% else %}{% static logo.img_file_src %}{% endif %}" | ||
| {% if logo.img_crossorigin %} | ||
| crossorigin="{{ logo.img_crossorigin }}" | ||
| {% endif %} | ||
| {% if logo.img_alt_text %} | ||
| alt="{{ logo.img_alt_text }}" | ||
| {% endif %} | ||
| /> | ||
| </a> | ||
| {% endwith %} | ||
|
|
||
| {% include "header_logo_via_settings.html" %} | ||
| {% endif %} |
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just copied over |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| {# @var className #} | ||
| {% load static custom_portal_settings %} | ||
|
|
||
| {% if settings.LOGO %} | ||
|
|
||
| {# via deprecated CMS setting (e.g. CMS developer sets logo image) #} | ||
| {% with settings.LOGO as logo %} | ||
| {% with filename=logo|index:1 selectors=logo|index:2 targeturl=logo|index:3 targettype=logo|index:4 accessibility=logo|index:5 corstype=logo|index:6 visibility=logo|index:7 %} | ||
| {% if visibility == "True" %} | ||
| <a id="header-logo" class="navbar-brand {{className}}" href="{{ targeturl }}" target="{{ targettype }}"> | ||
| <img class="portal-logo {{ selectors }}" src="{% static filename %}" crossorigin="{{ corstype }}" alt="{{ accessibility }}" /> | ||
| </a> | ||
| {% endif %} | ||
| {% endwith %} | ||
| {% endwith %} | ||
|
|
||
| {% elif settings.PORTAL_LOGO %} | ||
|
|
||
| {# via CMS setting (e.g. CMS developer sets logo image) #} | ||
| {% with settings.PORTAL_LOGO as logo %} | ||
| <a | ||
| id="header-logo" | ||
| class="navbar-brand {{className}}" | ||
| href="{{ logo.link_href }}" | ||
| {% if logo.link_target %} | ||
| target="{{ logo.link_target }}" | ||
| {% endif %} | ||
| {% if logo.link_name %} | ||
| aria-label="{{ logo.link_name }}" | ||
| {% endif %} | ||
| > | ||
| <img | ||
| class="portal-logo {{ logo.img_class }}" | ||
| src="{% if logo.is_remote %}{{ logo.img_file_src }}{% else %}{% static logo.img_file_src %}{% endif %}" | ||
| {% if logo.img_crossorigin %} | ||
| crossorigin="{{ logo.img_crossorigin }}" | ||
| {% endif %} | ||
| {% if logo.img_alt_text %} | ||
| alt="{{ logo.img_alt_text }}" | ||
| {% endif %} | ||
| /> | ||
| </a> | ||
| {% endwith %} | ||
|
|
||
| {% endif %} |
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a tag lib with one tag that render header logo, if any. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| from django import template | ||
| from django.utils.safestring import mark_safe | ||
|
|
||
| from taccsite_cms.contrib.taccsite_header_logo.render import render as render_header | ||
|
|
||
| register = template.Library() | ||
|
|
||
|
|
||
| @register.simple_tag(takes_context=True) | ||
| def render_header_logo(context): | ||
| html = render_header(context['request'], context) | ||
| return mark_safe(html) if html else '' |
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.
Create "Header logo" plugin (extends "Picture" plugin) so we can set defaults1.
Footnotes
Defaults are set in another file,
forms.py. ↩