MENDELU/Reduce the footer vendor credit to the company name - #1464
Conversation
The footer credit read "Theme by" stacked above "+ dataquest". Drop the wording entirely and leave the company name on its own. The i18n key footer.theme.by.message is now unused and removed from en.json5 and cs.json5. Nothing in the credit is translated any more, so English and Czech render identically. Layout notes: - The stacking came from a block-level <p> inside the narrow col-md-2 cell. The credit is now a single line of text with white-space: nowrap. - The whole credit is guarded by @if on the company name, the same way the base footer component does. Without it the anchor renders as <a href=""> on stacks that do not expose themed.by.company.name — an empty link with no discernible text, which fails the footer a11y e2e test (link-name). - *ngVar is no longer needed, so VarDirective drops out of the component's imports; it had no other use in this template. - The redundant [title] tooltip is dropped; the link text already supplies the accessible name. - Footer height is unchanged at 83px. Refs dataquest-dev/dspace-customers#592 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the custom theme footer vendor credit to render only the backend-provided company name (single line), removing the translated “Theme by” label and simplifying the template/layout accordingly.
Changes:
- Replaced
*ngVarusage with Angular control flow (@if) and removedVarDirectivefrom the themed footer component. - Added dedicated
.footer-sign/.dtq-signstyling for the vendor credit. - Removed the now-unused
footer.theme.by.messagetranslation key from EN/CS dictionaries.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/themes/custom/app/footer/footer.component.ts | Drops VarDirective from standalone imports after moving to @if. |
| src/themes/custom/app/footer/footer.component.html | Renders vendor credit as a single guarded line using @if. |
| src/themes/custom/app/footer/footer.component.scss | Introduces .footer-sign styles and link focus/hover behavior. |
| src/assets/i18n/en.json5 | Removes unused footer.theme.by.message key. |
| src/assets/i18n/cs.json5 | Removes unused footer.theme.by.message key (and related commented line). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Addresses review feedback on the footer credit.
When themed.by.company.name resolves but themed.by.url does not, the
anchor rendered as <a href="">, a link that goes nowhere and reloads the
current page on click. Render the name as plain text in that case and keep
the anchor for when the URL is actually available. Verified by returning
404 for themed.by.url alone: the credit stays visible, no anchor is
emitted.
Also set an explicit margin: 0 on .footer-sign. The theme's own
`footer p { margin: 0 }` already neutralises Bootstrap Reboot's
`p { margin-bottom: 1rem }`, so nothing was broken, but pinning it locally
keeps the bar height independent of a rule declared far away.
Refs dataquest-dev/dspace-customers#592
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Both review comments addressed in 5d4fef8. I reproduced each one against a real build rather than reasoning about it, by making the local proxy return 404 for individual config properties. 1. Angular does not stringify <a rel="noopener" class="dtq-sign" href="">+ dataquest</a>The DOM Fixed by linking only when the URL exists, and falling back to plain text otherwise: @if ((themedByUrl$ | async)?.payload?.values?.[0]; as companyUrl) {
<a class="dtq-sign" [href]="companyUrl" rel="noopener">{{ companyName }}</a>
} @else {
{{ companyName }}
}Dropping the credit entirely would have been the simpler guard, but the attribution is the point of the element — it still reads fine unlinked. (For reference, the base DSpace footer handles this with Verified with 2. The theme's own stylesheet declares That rule lives outside this diff, though, so the concern was fair from the diff alone. I added an explicit Verification matrix (compiled build, live dev-6 backend):
Footer height stays 83px in all three. |
milanmajchrak
left a comment
There was a problem hiding this comment.
The dataquest is not bold on the https://repozitar.mendelu.cz/
Review feedback: the credit came out bold, which production does not do. The .footer-sign block was carrying font-size: 1.0625rem, font-weight: 600 and letter-spacing from an earlier iteration of this change, when the credit was being redesigned around a copyright sign. That design was dropped in favour of the plain company name, but the type overrides stayed behind and silently restyled the credit. Measured on https://repozitar.mendelu.cz/ the credit renders at 16px, weight 400, letter-spacing normal. With those three declarations removed it inherits exactly that again, verified on the compiled build. What remains in the block is layout only, not looks: margin: 0 against Bootstrap Reboot, white-space: nowrap, the colour that replaces the removed text-white class, and the hover/focus states. Refs dataquest-dev/dspace-customers#592 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
@milanmajchrak good catch — fixed in bac855f. You were right, and the cause was a leftover. The Measured on https://repozitar.mendelu.cz/ today:
What is left in
Footer height stays 83px, same as before the change. Worth noting for the rest of the rollout: this only affected MENDELU. On TUL (#1466), ZCU (#1467, #1468) and the remaining branches the change is a plain deletion of the |


Drops the
Theme bywording from the footer's vendor credit and leaves the company name on its own.Part of dataquest-dev/dspace-customers#592. Supersedes #1457, which used a copyright sign — the team voted for the plain name instead. MENDELU first; once this is approved the same change goes out to the remaining seven customers.
Before / after
Theme byon its own line,+ dataqueststacked underneath+ dataquest, single line, nothing elseScreenshot below.
What changed
footer.component.html— the credit was a block-level<p>plus an<a>, which is what forced the two halves onto separate lines in the narrowcol-md-2cell. It is now one line, guarded on the company name:@if ((themedByCompanyName$ | async)?.payload?.values?.[0]; as companyName) { <p class="footer-sign"><a class="dtq-sign" […]>{{ companyName }}</a></p> }The guard matters: without it the anchor renders as
<a href=""></a>on any stack that does not exposethemed.by.company.name— including CI — which is an empty link with no discernible text and fails the footer a11y e2e test on thelink-namerule.footer.component.scss— new.footer-signblock..footer-sign,.theme-byand.dtq-signhad no rules of their own before; the layout came from Bootstrap utilities alone.footer.component.ts—VarDirectiveremoved from imports.*ngVarwas only used for this one credit and the@ifreplaces it.en.json5/cs.json5—footer.theme.by.messageremoved, now unused. Both files still parse (3698 / 3695 keys).Verification
Run locally against the live dev-6 backend (real MENDELU theme and data), on the compiled build, in both languages:
<a rel="noopener" class="dtq-sign" href="https://www.dataquest.sk/dspace">+ dataquest</a>+ dataquest(EN and CS identical)#fff,white-space: nowrapAlso checked the empty case by making the proxy return 404 for
themed.by.company.name: the credit does not render at all, and no empty anchor is left behind.Notes for the reviewer
+in+ dataquestcomes from the backend propertythemed.by.company.nameand is untouched. Removing it would be a backend change, not a frontend one.#fffon this green measures 2.29:1, below the WCAG AA threshold of 4.5:1 — but the nav links next to it already have the same problem, so this PR neither introduces nor fixes it. Better as a separate ticket than as a visual change smuggled in here.🤖 Generated with Claude Code