Skip to content

Fix NextJS 16 warnings about logos#8052

Merged
gilluminate merged 3 commits into
mainfrom
gill/fix/nextjs-warning-about-logo
Apr 29, 2026
Merged

Fix NextJS 16 warnings about logos#8052
gilluminate merged 3 commits into
mainfrom
gill/fix/nextjs-warning-about-logo

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

no ticket

Description Of Changes

NextJS builds throw warnings about logos being the first big paint job and recommend adding this prop to load them faster since they are above the fold.

Code Changes

  • Add loading="eager" to login and main logos

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Apr 28, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Apr 29, 2026 11:32am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Apr 29, 2026 11:32am

Request Review

@gilluminate gilluminate changed the title Fix NextJS warnings about logos Fix NextJS 16 warnings about logos Apr 28, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.31% (2800/44305) 5.56% (1404/25229) 4.41% (579/13106)
fides-js Coverage: 78%
79.39% (2011/2533) 65.99% (1240/1879) 73.09% (345/472)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

@gilluminate gilluminate marked this pull request as ready for review April 28, 2026 20:35
@gilluminate gilluminate requested a review from a team as a code owner April 28, 2026 20:35
@gilluminate gilluminate requested review from lucanovera and removed request for a team April 28, 2026 20:35
@rayharnett
Copy link
Copy Markdown
Contributor

Code Review: PR #8052 — Fix NextJS 16 warnings about logos

Author: @gilluminate
Base: maingill/fix/nextjs-warning-about-logo
Files changed: 2 (login.tsx, forgot-password.tsx)


🚨 Critical Issues (Must Fix)

None.


⚠️ Warnings (Should Fix)

1. Missing changelog entry

The "Check Changelog Entry" check is failing. While this is a minor frontend tweak, the project's changelog process appears to require an entry for every PR. Consider adding a line to CHANGELOG.md (e.g., under an ## [Unreleased] or ## [Next] section) such as:

### Fixed
- Eliminated Next.js 16 warnings for eager-loading above-the-fold logos on login and forgot-password pages

💡 Suggestions (Nitpicks/Improvements)

1. Consider a shared logo component

Both login.tsx and forgot-password.tsx have identical Image markup with loading="eager". If other pages also render the same hero logo, consider extracting a shared Logo component (e.g., clients/admin-ui/src/components/Logo.tsx) to:

  • Avoid duplication
  • Centralize props (width, height, alt, loading)
  • Make future tweaks (e.g., switching to next/future/image or adding priority) a single-line change
// clients/admin-ui/src/components/Logo.tsx
import { Image } from 'antd';

export const Logo = () => (
  <Image
    src="/logo.svg"
    alt="Fides logo"
    width={205}
    height={46}
    loading="eager"
  />
);

2. No tests needed

This is a styling/behavior change with no new logic. The existing Cypress e2e tests cover these pages. No test additions required.


✅ Good Practices

  • Correct fix: loading="eager" is the right Next.js 16 approach for above-the-fold images. This aligns with Next.js's own recommendation to replace priority with explicit eager loading.
  • Minimal diff: Only the necessary prop was added — no scope creep.
  • Clear commit message: Fix NextJS warnings about logos is concise and accurate.

Summary

Verdict: Approve (pending changelog entry). This is a straightforward, low-risk fix for a linter warning. The change is correct and minimal.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@gilluminate gilluminate added this pull request to the merge queue Apr 29, 2026
Merged via the queue into main with commit f482898 Apr 29, 2026
53 checks passed
@gilluminate gilluminate deleted the gill/fix/nextjs-warning-about-logo branch April 29, 2026 15:05
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.

2 participants