Skip to content

feat: max size for logos, no tiny branding#1121

Merged
wesleyboar merged 1 commit intomainfrom
feat/max-size-for-logos-and-no-tiny-branding
Mar 17, 2026
Merged

feat: max size for logos, no tiny branding#1121
wesleyboar merged 1 commit intomainfrom
feat/max-size-for-logos-and-no-tiny-branding

Conversation

@wesleyboar
Copy link
Member

Overview

Greater default size for branding logos (from snippet).

Related

Changes

  • added bix width and height constraints into SVG CSS
  • deleted tiny width and height constraints from SVG HTML

Testing

Dimensions of branding logos (from snippet) are constrained by their parent container or their max block size (500px).

UI

before after test max size
before after test max size

@wesleyboar wesleyboar merged commit 4420479 into main Mar 17, 2026
@qodo-code-review
Copy link

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Review Summary by Qodo

Add max-block-size constraint and remove inline SVG dimensions

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Remove inline width/height constraints from SVG elements
• Add max-block-size CSS constraint to prevent oversized logos
• Allow logos to scale responsively within 500px maximum
Diagram
flowchart LR
  A["SVG with inline<br/>width/height=1em"] -->|Remove inline<br/>attributes| B["SVG without<br/>inline dimensions"]
  B -->|Apply CSS<br/>max-block-size| C["Logo constrained<br/>to 500px max"]
Loading

Grey Divider

File Changes

1. taccsite_cms/templates/snippets/logos-branding.html ✨ Enhancement +3/-3

Remove inline SVG width/height attributes

• Removed width="1em" and height="1em" attributes from three SVG elements
• Allows SVG sizing to be controlled by CSS instead of inline styles
• Affects TACC, NSF, and University of Texas logos

taccsite_cms/templates/snippets/logos-branding.html


2. taccsite_cms/static/site_cms/css/src/_imports/components/logos.css ✨ Enhancement +1/-0

Add maximum size constraint to logo SVGs

• Added max-block-size: 500px CSS property to SVG elements
• Prevents logos from becoming unexpectedly large
• Allows responsive scaling while maintaining size constraints

taccsite_cms/static/site_cms/css/src/_imports/components/logos.css


Grey Divider

Qodo Logo

@wesleyboar wesleyboar deleted the feat/max-size-for-logos-and-no-tiny-branding branch March 17, 2026 03:39
@qodo-code-review
Copy link

qodo-code-review bot commented Mar 17, 2026

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Action required

1. SVG size becomes default 🐞 Bug ✓ Correctness
Description
logos-branding.html removes explicit SVG dimensions but logos.css does not define any
replacement width/height/inline-size, so branding logos fall back to UA default sizing rather than
scaling with site styling. This can change layout deterministically (e.g., no longer tied to 1em)
and makes sizing control less reliable across contexts.
Code

taccsite_cms/templates/snippets/logos-branding.html[3]

+        <svg role="img">
Evidence
The branding snippet’s <svg> elements no longer specify width/height, and the only shared
styling for all [class*="logos--"] a svg sets display and max-block-size—no base width/height
is defined. Other logo usages in the same codebase still explicitly use width="1em" height="1em",
indicating the expected pattern is to control icon sizing rather than relying on UA defaults.

taccsite_cms/templates/snippets/logos-branding.html[1-19]
taccsite_cms/static/site_cms/css/src/_imports/components/logos.css[3-9]
taccsite_cms/templates/snippets/logos-social-media.html[1-7]
taccsite_cms/templates/share_on_social.html[13-18]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Branding logos no longer have explicit SVG dimensions, and CSS does not define a replacement width/height/inline-size. This makes branding logo sizing depend on UA defaults instead of being controlled by the site.

### Issue Context
Other logo usages in the codebase still use `width=&quot;1em&quot; height=&quot;1em&quot;`, indicating the expected pattern is explicit sizing rather than UA-default sizing.

### Fix Focus Areas
- taccsite_cms/templates/snippets/logos-branding.html[1-19]
- taccsite_cms/static/site_cms/css/src/_imports/components/logos.css[3-9]

### Implementation notes
- Prefer CSS-based sizing for `.logos--branding a svg` (or similar) to keep HTML clean.
- Ensure there is a *base* size (e.g., `inline-size: ...`) plus caps, and preserve aspect ratio (`block-size: auto`).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. target=_blank missing rel 🐞 Bug ⛨ Security
Description
The branding snippet opens external links with target="_blank" but omits `rel="noopener
noreferrer", allowing the destination page to access window.opener` (reverse-tabnabbing). This is
a concrete, known security issue for the NSF and UT links.
Code

taccsite_cms/templates/snippets/logos-branding.html[8]

    <a href="https://www.nsf.gov/" target="_blank" class="logos__nsf">
-        <svg width="1em" height="1em" role="img">
Evidence
Both external anchors in the modified snippet include target="_blank" but have no rel attribute,
which exposes window.opener to the newly opened page.

taccsite_cms/templates/snippets/logos-branding.html[8-19]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
External links using `target=&quot;_blank&quot;` without `rel=&quot;noopener noreferrer&quot;` allow reverse-tabnabbing via `window.opener`.

### Issue Context
The branding snippet includes external links to NSF and UT opened in a new tab.

### Fix Focus Areas
- taccsite_cms/templates/snippets/logos-branding.html[8-19]

### Implementation notes
- Add `rel=&quot;noopener noreferrer&quot;` to the affected anchors.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Wrong NSF accessible name 🐞 Bug ✓ Correctness
Description
The NSF logo <svg> uses <title>Bluesky</title>, so assistive technology will announce the NSF
logo/link as “Bluesky”. This is a correctness/accessibility defect that mislabels the content and
destination.
Code

taccsite_cms/templates/snippets/logos-branding.html[10]

            <title>Bluesky</title>
Evidence
Within the NSF anchor (logos__nsf), the <use> references nsf-logo.svg, but the adjacent
<title> incorrectly says “Bluesky,” which becomes the accessible name for the SVG in many AT
flows.

taccsite_cms/templates/snippets/logos-branding.html[8-12]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The NSF branding logo SVG is mislabeled with `&lt;title&gt;Bluesky&lt;/title&gt;`, producing an incorrect accessible name.

### Issue Context
The SVG `&lt;use&gt;` references `nsf-logo.svg`, so the title should describe NSF.

### Fix Focus Areas
- taccsite_cms/templates/snippets/logos-branding.html[8-12]

### Implementation notes
- Replace the `&lt;title&gt;` text with an accurate label, e.g., `NSF (National Science Foundation)`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant