Skip to content

fix: spacing for blank links in header and footer - #1302

Open
a-friedman wants to merge 4 commits into
mainfrom
header-links
Open

fix: spacing for blank links in header and footer#1302
a-friedman wants to merge 4 commits into
mainfrom
header-links

Conversation

@a-friedman

@a-friedman a-friedman commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

context: https://yext.slack.com/archives/C02UVSE7P6W/p1787940041327369

Updates the HeaderLinks component to not have inconsistent spacing when one or more of the links are blank. This is output straight from codex that I didn't review very closely, but appeared to work correctly when testing in platform dev mode.

Screen.Recording.2026-08-31.at.11.29.14.mov

@coderabbitai

coderabbitai Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 3499dad3-d43d-4774-9b64-1e7b2e409499


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information.

@nanhu95
nanhu95 marked this pull request as ready for review August 31, 2026 15:41
@nanhu95

nanhu95 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Warning: Component files have been updated but no migrations have been added. See https://github.com/yext/visual-editor/blob/main/packages/visual-editor/src/components/migrations/README.md for more information.

I think this is a false positive since props aren't being changed

@asanehisa asanehisa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think we'll want a similar fix for the footer links

Comment thread packages/visual-editor/src/components/header/HeaderLinks.test.tsx Outdated
@nanhu95

nanhu95 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

i think we'll want a similar fix for the footer links

added in new commit

@nanhu95 nanhu95 changed the title fix: fix spacing for blank links in header fix: fix spacing for blank links in header and footer Aug 31, 2026
@nanhu95
nanhu95 requested a review from asanehisa August 31, 2026 19:50

@benlife5 benlife5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit; don't duplicate "fix" in the title, just do
"fix: spacing for ..."

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would put this file in the components/footer directory since it's footer-specific (but resolveLocalizedCtas makes sense here)

import { type TranslatableCTA } from "../types/types.ts";
import { resolveComponentData } from "./resolveComponentData.tsx";

export type ResolvedCTA = Omit<TranslatableCTA, "label" | "link"> & {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this type might be equivalent to import { CTA as CTAType } from "@yext/pages-components";

resolveLocalizedCtas,
} from "./resolveLocalizedCtas.ts";

type FooterSection = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe FooterLinkSection?

@nanhu95 nanhu95 changed the title fix: fix spacing for blank links in header and footer fix: spacing for blank links in header and footer Aug 31, 2026
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.

4 participants