EuiHeaderLogo new logoType prop - #9875
Conversation
10 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiheader (8 differences)
euicollapsiblenav (2 differences)
|
14 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiheader (3 differences)
euidatagrid (11 differences)
|
14 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiheader (3 differences)
euidatagrid (11 differences)
|
13 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiheader (3 differences)
euidatagrid (10 differences)
|
|
Hey @weronikaolejniczak I'd like to get your help on this. I decided to conditionally render either an EuiIcon or embedded SVG to display the glyph logo or the full logo. {logoType === 'glyph' && (
<EuiIcon
aria-label={iconTitle}
className="euiHeaderLogo__icon"
size="l"
type={iconType}
/>
)}
{logoType === 'horizontal' && (
<SvgElasticLogoFull
title={iconTitle}
style={{ maxHeight: euiTheme.size.l, width: 'auto' }}
className="euiHeaderLogo__image"
/>
)}The EuiIcon constrains the size to always a box container. And it changes based on the theme applied as shown here: 
But when we force the EuiHeader with theme={"dark"} looks like it doesn't affect the EuiHeraderLogo content and doesn't force it to behave like always dark as I'd expect: 
|
|
@JoseLuisGJ we'd need to make the SVG use .euiHeaderLogo__text,
.euiHeaderLogo__image, // <- add this
.euiHeaderLink,
.euiHeaderSectionItemButton {
color: ${euiTheme.colors.textGhost};
}The header deliberately doesn't use |
|
@JoseLuisGJ we'd need to make the SVG use .euiHeaderLogo__text,
.euiHeaderLogo__image, // <- add this
.euiHeaderLink,
.euiHeaderSectionItemButton {
color: ${euiTheme.colors.textGhost};
}The header deliberately doesn't use |
|
Thanks @weronikaolejniczak now it also applies properly the theme dark to the header in Docs: 
|
There was a problem hiding this comment.
Pull request overview
This PR updates EuiHeaderLogo to support a new logoType prop that switches between the existing Elastic glyph and a new inline horizontal full-wordmark SVG, and updates EUI docs/stories/tests to reflect the new usage.
Changes:
- Added a new
logoType?: 'glyph' | 'horizontal'API and horizontal SVG render path forEuiHeaderLogo - Introduced an inline
SvgElasticLogoFullcomponent and updated styling for dark headers - Updated Storybook stories, a11y example, docs examples, and Jest snapshots for the new output
Reviewed changes
Copilot reviewed 12 out of 63 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/website/docs/components/layout/header.mdx | Updates documentation examples to use logoType="horizontal" and removes iconType/children usage. |
| packages/eui/src/components/header/header.styles.ts | Ensures dark header styling applies to the new horizontal logo element class. |
| packages/eui/src/components/header/header.stories.tsx | Updates header stories to stop passing iconType/children and demonstrate horizontal logo usage. |
| packages/eui/src/components/header/header.a11y.tsx | Updates the header a11y example to no longer pass iconType. |
| packages/eui/src/components/header/header_logo/header_logo.tsx | Implements the logoType switch and renders the new horizontal SVG variant. |
| packages/eui/src/components/header/header_logo/header_logo.test.tsx | Updates unit tests to reflect new rendering behavior and adds an iconType legacy assertion. |
| packages/eui/src/components/header/header_logo/header_logo.styles.ts | Removes text styling now that text children rendering is removed. |
| packages/eui/src/components/header/header_logo/header_logo.stories.tsx | Updates stories to demo glyph vs horizontal variants. |
| packages/eui/src/components/header/header_logo/elastic_logo_full.tsx | Adds the new inline horizontal full-wordmark SVG component. |
| packages/eui/src/components/header/header_logo/snapshots/header_logo.test.tsx.snap | Updates snapshots for the new rendered markup (now includes the inline SVG). |
| packages/eui/src/components/collapsible_nav/collapsible_nav.stories.tsx | Updates an example to use the horizontal logo variant. |
| packages/eui/changelogs/upcoming/9875.md | Adds a changelog entry describing the new logoType prop. |
Suppressed comments (1)
packages/eui/src/components/header/header_logo/header_logo.test.tsx:56
renders optional propsnow snapshots the entire inline horizontal SVG, which makes the snapshot very large and brittle (minor SVG output changes will cause big diffs). Consider replacing the snapshot with targeted assertions (e.g..euiHeaderLogo__imageexists, a<title>containsiconTitle, and styles are applied), and deleting/updating the corresponding snapshot entry.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
44 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euilistgroup (2 differences)
euititle (1 difference)
euiexpression (3 differences)
euiform (7 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euidatagrid (6 differences)
euitable (1 difference)
euithemeprovider (4 differences)
|
44 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euilistgroup (2 differences)
euititle (1 difference)
euiexpression (3 differences)
euiform (7 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euidatagrid (6 differences)
euitable (1 difference)
euithemeprovider (4 differences)
|
44 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euilistgroup (2 differences)
euititle (1 difference)
euiexpression (3 differences)
euiform (7 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euidatagrid (6 differences)
euitable (1 difference)
euithemeprovider (4 differences)
|
45 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euilistgroup (2 differences)
euititle (1 difference)
euiexpression (3 differences)
euiform (7 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euidatagrid (7 differences)
euitable (1 difference)
euithemeprovider (4 differences)
|
92 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euibadge (5 differences)
euicallout (9 differences)
euilistgroup (2 differences)
euititle (1 difference)
euitour (1 difference)
euicombobox (1 difference)
euiexpression (3 differences)
euifiltergroup (3 differences)
euiform (8 differences)
euirange (7 differences)
euisuperdatepicker (9 differences)
euilistitemlayout (10 differences)
euibottombar (1 difference)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euiinputpopover (3 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euisidenav (2 differences)
euidatagrid (6 differences)
euiinmemorytable (2 differences)
euitable (1 difference)
euiprovider (2 differences)
euithemeprovider (4 differences)
|
|
@JoseLuisGJ could you rebase onto main? This should fix the weird VRTs. |
44 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuiaspectratio (1 difference)
euilistgroup (2 differences)
euititle (1 difference)
euiexpression (3 differences)
euiform (7 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiheader (4 differences)
euipage (1 difference)
euicontextmenu (3 differences)
euidatagrid (6 differences)
euitable (1 difference)
euithemeprovider (4 differences)
|
54bd25e to
3282530
Compare
3282530 to
58a4628
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/eui/src/components/header/header_logo/header_logo.tsx:28
- Removing
iconTypefrom the public props leaves an existing repository usage atpackages/website/docs/components/display/progress.mdx:181-185passingiconType="logoKibana". That example now uses an invalid prop and, at runtime, silently renders the Elastic glyph instead of the Kibana logo. Update that example as part of this API migration.
export type EuiHeaderLogoProps = CommonProps &
Omit<AnchorHTMLAttributes<HTMLAnchorElement>, 'children'> & {
href?: string;
packages/eui/src/components/header/header_logo/header_logo.stories.tsx:41
- This story still appears in Storybook as “With Text”, although the changed API removes text children and the story now demonstrates the horizontal logo. Rename it to
Horizontalso the story does not imply that text content remains supported.
args: {
logoType: 'horizontal',
packages/eui/changelogs/upcoming/9875.md:1
- This entry is formatted only as an addition and omits the removal of
iconType, even though this PR removes bothiconTypeandchildren. The changelog template records removals under**Breaking changes**(packages/eui/changelogs/upcoming/_template.md:11-13); document both removals there so consumers receive complete migration information.
- Added new `logoType` prop to the `EuiHeaderLogo` component to render either the `glyph` or `horizontal` Elastic logo versions. `children` text content is no longer rendered.
💚 Build Succeeded
History
|
|
Copilot's suppressed comments in: #9875 (review) are actually worth tackling |
65 visual difference(s) found - expand to review, then click Approve visual changes to update baselineseuicomment (2 differences)
euidroppable (2 differences)
euipinnablelistgroup (1 difference)
euitimeline (2 differences)
euititle (1 difference)
euicolorpicker (4 differences)
euiform (2 differences)
euiradiogroup (2 differences)
euiselectable (4 differences)
euisuperdatepicker (9 differences)
euiconfirmmodal (1 difference)
euiflexitem (1 difference)
euiflyout (1 difference)
euiheader (11 differences)
euiinputpopover (3 differences)
euipopover (4 differences)
euicollapsiblenav (3 differences)
euisteps (2 differences)
euidatagrid (8 differences)
euiautosizer (1 difference)
euioutsideclickdetector (1 difference)
|
💔 Build Failed
Failed CI StepsHistory
|
Summary
EuiHeaderLogoto support a newlogoTypeAPI that switches between the Elastic glyph logo and a new horizontal full-wordmark logo. Added a dedicated inline SVG component for the horizontal logo and updated stories/tests/snapshots accordingly.logoType?: 'glyph' | 'horizontal'toEuiHeaderLogoin sync with the naming convention if Elastic branding.SvgElasticLogoFullinline SVG and render path whenlogoType="horizontal".API Changes
glyph(default) orhorizontal.Screenshots
logoType="glyph"orlogoType="horizontal"Docs
Impact Assessment
Note: Most PRs should be tested in Kibana to help gauge their Impact before merging.
iconTypeandchildrenwere removed fromEuiHeaderLogo. Any consumer using custom icon types or inline text content will need updates.Impact level: 🟡 Moderate
Release Readiness
EuiHeaderLogonewlogoTypeprop and removediconType/children.iconTypeusage withlogoTypewhere applicable.EuiHeaderLogo.logoType="horizontal"where full wordmark is desired.QA instructions for reviewer
logoType="glyph"still renders expected Elastic glyph size and alignment.logoType="horizontal"renders the new full logo at expected dimensions.iconTitleis announced correctly in glyph and horizontal modes.href,rel,target, secure rel behavior.Checklist before marking Ready for Review
breaking changelabel (if applicable)Reviewer checklist