Skip to content

feat(Masthead, PageHeader): update component logic #8655

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Feb 10, 2023
Merged

feat(Masthead, PageHeader): update component logic #8655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d2c6bec
feat(Masthead, PageHeader): update component logic
kmcfaul Feb 6, 2023
55e1bf4
remove button branch, update tabindex
kmcfaul Feb 9, 2023
db8cf3b
another fix for DropdownToggleCheckbox
kmcfaul Feb 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion packages/react-core/src/components/Dropdown/DropdownToggleCheckbox.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ export class DropdownToggleCheckbox extends React.Component<DropdownToggleCheckb
const spinner = (
<Spinner
diameter="1em"
isSVG
aria-valuetext={defaultProgressAriaValueText}
aria-live="polite"
aria-label={defaultProgressAriaLabel}
Expand Down
13 changes: 10 additions & 3 deletions packages/react-core/src/components/Masthead/MastheadBrand.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ export interface MastheadBrandProps
export const MastheadBrand: React.FunctionComponent<MastheadBrandProps> = ({
children,
className,
component = 'a',
component,
...props
}: MastheadBrandProps) => {
const Component = component as any;
let Component = component as any;
if (!component) {
if (props?.href !== undefined) {
Component = 'a';
} else {
Component = 'span';
}
}
return (
<Component className={css(styles.mastheadBrand, className)} tabIndex={0} {...props}>
<Component className={css(styles.mastheadBrand, className)} {...(Component === 'a' && { tabIndex: 0 })} {...props}>
{children}
</Component>
);
Expand Down
12 changes: 12 additions & 0 deletions packages/react-core/src/components/Masthead/__tests__/Masthead.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,23 @@ describe('MastheadBrand', () => {
expect(asFragment()).toMatchSnapshot();
});

test('verify default component', () => {
const { asFragment } = render(<MastheadBrand>test</MastheadBrand>);

expect(asFragment()).toMatchSnapshot();
});

test('verify custom component', () => {
const { asFragment } = render(<MastheadBrand component="div">test</MastheadBrand>);

expect(asFragment()).toMatchSnapshot();
});

test('verify anchor component with href', () => {
const { asFragment } = render(<MastheadBrand href="#">test</MastheadBrand>);

expect(asFragment()).toMatchSnapshot();
});
});

describe('MastheadContent', () => {
Expand Down
34 changes: 26 additions & 8 deletions packages/react-core/src/components/Masthead/__tests__/__snapshots__/Masthead.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,11 @@ exports[`Masthead verify full structure 1`] = `
<div
class="pf-c-masthead__main"
>
<a
<span
class="pf-c-masthead__brand"
tabindex="0"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-masthead__content"
Expand Down Expand Up @@ -151,39 +150,58 @@ exports[`Masthead verify stack display breakpoints 1`] = `
</DocumentFragment>
`;

exports[`MastheadBrand verify basic 1`] = `
exports[`MastheadBrand verify anchor component with href 1`] = `
<DocumentFragment>
<a
class="pf-c-masthead__brand"
href="#"
tabindex="0"
>
test
</a>
</DocumentFragment>
`;

exports[`MastheadBrand verify basic 1`] = `
<DocumentFragment>
<span
class="pf-c-masthead__brand"
>
test
</span>
</DocumentFragment>
`;

exports[`MastheadBrand verify custom class 1`] = `
<DocumentFragment>
<a
<span
class="pf-c-masthead__brand custom-css"
tabindex="0"
>
test
</a>
</span>
</DocumentFragment>
`;

exports[`MastheadBrand verify custom component 1`] = `
<DocumentFragment>
<div
class="pf-c-masthead__brand"
tabindex="0"
>
test
</div>
</DocumentFragment>
`;

exports[`MastheadBrand verify default component 1`] = `
<DocumentFragment>
<span
class="pf-c-masthead__brand"
>
test
</span>
</DocumentFragment>
`;

exports[`MastheadContent verify basic 1`] = `
<DocumentFragment>
<div
Expand Down
15 changes: 11 additions & 4 deletions packages/react-core/src/components/Page/PageHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export interface PageHeaderProps extends React.HTMLProps<HTMLDivElement> {
/** Component to render the logo/brand, use <Brand /> */
logo?: React.ReactNode;
/** Additional props passed to the logo anchor container */
logoProps?: object;
logoProps?: any;
/** Component to use to wrap the passed <logo> */
logoComponent?: React.ReactNode;
/** Component to render the header tools, use <PageHeaderTools /> */
Expand All @@ -36,8 +36,8 @@ export interface PageHeaderProps extends React.HTMLProps<HTMLDivElement> {
export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
className = '',
logo = null as React.ReactNode,
logoProps = null as object,
logoComponent = 'a',
logoProps = null as any,
logoComponent,
headerTools = null as React.ReactNode,
topNav = null as React.ReactNode,
isNavOpen = true,
Expand All @@ -49,7 +49,14 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
'aria-controls': ariaControls = null,
...props
}: PageHeaderProps) => {
const LogoComponent = logoComponent as any;
let LogoComponent = logoComponent as any;
if (!logoComponent) {
if (logoProps?.href !== undefined) {
LogoComponent = 'a';
} else {
LogoComponent = 'span';
}
}
return (
<PageContextConsumer>
{({ isManagedSidebar, onNavToggle: managedOnNavToggle, isNavOpen: managedIsNavOpen }: PageContextProps) => {
Expand Down
21 changes: 21 additions & 0 deletions packages/react-core/src/components/Page/__tests__/PageHeader.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,24 @@ test('Check page vertical layout example against snapshot', () => {
const { asFragment } = render(Header);
expect(asFragment()).toMatchSnapshot();
});

test('Test that passed logoComponent overrides default', () => {
const Header = (
<PageHeader logo="Logo" logoComponent="div" headerTools="PageHeaderTools | Avatar" onNavToggle={() => undefined} />
);
const { asFragment } = render(Header);
expect(asFragment()).toMatchSnapshot();
});

test('Test that logoComponent with href is an anchor', () => {
const Header = (
<PageHeader
logo="Logo"
logoProps={{ href: '#' }}
headerTools="PageHeaderTools | Avatar"
onNavToggle={() => undefined}
/>
);
const { asFragment } = render(Header);
expect(asFragment()).toMatchSnapshot();
});
52 changes: 26 additions & 26 deletions packages/react-core/src/components/Page/__tests__/__snapshots__/Page.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ exports[`Page Check dark page against snapshot 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
PageHeaderTools | Avatar
</header>
Expand Down Expand Up @@ -74,11 +74,11 @@ exports[`Page Check page horizontal layout example against snapshot 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -138,11 +138,11 @@ exports[`Page Check page to verify breadcrumb is created - PageBreadcrumb syntax
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -305,11 +305,11 @@ exports[`Page Check page to verify breadcrumb is created 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -472,11 +472,11 @@ exports[`Page Check page to verify grouped nav and breadcrumb - new components s
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -758,11 +758,11 @@ exports[`Page Check page to verify grouped nav and breadcrumb - old / props synt
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1049,11 +1049,11 @@ exports[`Page Check page to verify nav is created - PageNavigation syntax 1`] =
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1235,11 +1235,11 @@ exports[`Page Check page to verify skip to content points to main content region
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1403,11 +1403,11 @@ exports[`Page Check page vertical layout example against snapshot 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
PageHeaderTools | Avatar
</header>
Expand Down Expand Up @@ -1464,11 +1464,11 @@ exports[`Page Sticky bottom breadcrumb on all height breakpoints - PageBreadcrum
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1631,11 +1631,11 @@ exports[`Page Verify sticky bottom breadcrumb on all height breakpoints 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1798,11 +1798,11 @@ exports[`Page Verify sticky top breadcrumb on all height breakpoints - PageBread
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down Expand Up @@ -1965,11 +1965,11 @@ exports[`Page Verify sticky top breadcrumb on all height breakpoints 1`] = `
<div
class="pf-c-page__header-brand"
>
<a
<span
class="pf-c-page__header-brand-link"
>
Logo
</a>
</span>
</div>
<div
class="pf-c-page__header-nav"
Expand Down
Loading