Skip to content

Conversation

@rkeerthient
Copy link
Contributor

Ticket - OPAQF-57

  • Added a min-height when there is no image in header logo.
  • Updated test classes.
Screen.Recording.2026-01-02.at.5.21.16.PM.mov

@github-actions
Copy link
Contributor

github-actions bot commented Jan 2, 2026

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.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

Walkthrough

Adds a test "version 49 props - no logo" to ExpandedHeader.test.tsx covering a primary header without a logo image. Updates PrimaryHeaderSlot to detect whether LogoSlot contains image data and expose that as conditionalRender.hasLogoImage from resolveData; PrimaryHeaderSlotProps now includes hasLogoImage?: boolean. The component wraps LogoSlot in a container that applies min-height: 100px when no logo image is present; when an image exists height remains automatic. package.json and starter/package.json only gained trailing newlines.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client/Renderer
    participant Resolver as resolveData
    participant Primary as PrimaryHeaderSlot
    participant Logo as LogoSlot

    rect rgb(247,250,255)
      Note over Client,Resolver: Render request lifecycle
    end

    Client->>Resolver: request header data
    Resolver-->>Client: resolved data (slots + conditionalRender)
    Client->>Primary: render header with resolved data
    Primary->>Logo: inspect LogoSlot content
    Logo-->>Primary: returns image data (present or absent)
    Primary->>Primary: set conditionalRender.hasLogoImage
    alt hasLogoImage == true
        Primary->>Logo: render LogoSlot (auto height)
    else hasLogoImage == false
        Primary->>Logo: render LogoSlot inside wrapper (min-height:100px)
    end
    Primary-->>Client: composed header DOM
Loading

Possibly related PRs

Suggested reviewers

  • asanehisa
  • briantstephan
  • mkilpatrick

Pre-merge checks and finishing touches

✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: adding min-height for the logo slot when no image is present, which is the core fix in PrimaryHeaderSlot.tsx.
Description check ✅ Passed The description is related to the changeset, mentioning the min-height addition for logo when no image exists and test updates, both of which are present in the PR.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 31e0295 and 632a718.

⛔ Files ignored due to path filters (3)
  • packages/visual-editor/src/components/testing/screenshots/EventSection/[tablet] default props with empty document.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 49 props - no logo.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 49 props - no logo.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (1)
  • packages/visual-editor/src/components/header/PrimaryHeaderSlot.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/visual-editor/src/components/header/PrimaryHeaderSlot.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: call_unit_test / unit_tests (22.x)
  • GitHub Check: call_unit_test / unit_tests (24.x)
  • GitHub Check: call_unit_test / unit_tests (20.x)
  • GitHub Check: semgrep/ci

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 and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

📜 Review details

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c4682d and d9b0191.

⛔ Files ignored due to path filters (29)
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 10 props - secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 10 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 11 props - secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 15 props - fixed header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 15 props - sticky header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 20 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 41 props - no secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 41 props - with secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 48 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[desktop] version 50 props - no logo.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] default props (after interactions).png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 10 props (after interactions).png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 10 props - secondary header (after interactions).png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 11 props - secondary header (after interactions).png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 15 props - fixed header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 15 props - sticky header (after interactions).png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 20 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[mobile] version 50 props - no logo.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] default props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 10 props - secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 11 props - secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 15 props - fixed header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 15 props - sticky header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 20 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 41 props - no secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 41 props - with secondary header.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 48 props.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
  • packages/visual-editor/src/components/testing/screenshots/ExpandedHeader/[tablet] version 50 props - no logo.png is excluded by !**/*.png, !packages/visual-editor/src/components/testing/screenshots/**
📒 Files selected for processing (2)
  • packages/visual-editor/src/components/header/ExpandedHeader.test.tsx
  • packages/visual-editor/src/components/header/PrimaryHeaderSlot.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: call_unit_test / unit_tests (24.x)
  • GitHub Check: call_unit_test / unit_tests (22.x)
  • GitHub Check: call_unit_test / unit_tests (20.x)
  • GitHub Check: semgrep/ci
🔇 Additional comments (2)
packages/visual-editor/src/components/header/PrimaryHeaderSlot.tsx (1)

110-118: Verify the logic after refactoring the image detection.

The wrapper structure and conditional minHeight logic align with the PR objectives. However, the effectiveness depends on correctly detecting image presence. Once the image detection approach is refactored (see comments on lines 91-92 and 97-105), verify that:

  1. The 100px minHeight is applied only when LogoSlot is empty or contains no image.
  2. No layout shift occurs during the initial render.
packages/visual-editor/src/components/header/ExpandedHeader.test.tsx (1)

943-1088: LGTM! Test case properly covers the no-logo scenario.

The new test case follows the established pattern of existing version tests and correctly validates the header behavior when LogoSlot is empty (line 960). This should exercise the conditional minHeight logic added in PrimaryHeaderSlot.tsx and ensure consistent spacing when no logo is present.

@benlife5 benlife5 merged commit 0df2427 into main Jan 5, 2026
17 checks passed
@benlife5 benlife5 deleted the OPAQF-57 branch January 5, 2026 20:44
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