Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion packages/react-catalog-view-extension/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@
"clean": "rimraf dist"
},
"dependencies": {
"@patternfly/patternfly": "4.10.20",
"@patternfly/patternfly": "4.10.21",
"@patternfly/react-core": "^4.13.1",
"@patternfly/react-styles": "^4.2.8",
"classnames": "^2.2.5",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-charts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
},
"homepage": "https://github.com/patternfly/patternfly-react#readme",
"dependencies": {
"@patternfly/patternfly": "4.10.20",
"@patternfly/patternfly": "4.10.21",
"@patternfly/react-styles": "^4.2.8",
"@patternfly/react-tokens": "^4.3.6",
"hoist-non-react-statics": "^3.3.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/react-core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@
"tslib": "^1.11.1"
},
"devDependencies": {
"@patternfly/patternfly": "4.10.20",
"@patternfly/patternfly": "4.10.21",
"@rollup/plugin-commonjs": "^11.0.2",
"@rollup/plugin-node-resolve": "^7.1.1",
"@rollup/plugin-replace": "^2.3.1",
Expand Down
20 changes: 6 additions & 14 deletions packages/react-core/src/components/Page/PageHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,15 @@ import { PageContextConsumer } from './Page';
export interface PageHeaderProps extends React.HTMLProps<HTMLDivElement> {
/** Additional classes added to the page header */
className?: string;
/** Component to render the logo/brand (e.g. <Brand />) */
/** Component to render the logo/brand, use <Brand /> */
logo?: React.ReactNode;
/** Additional props passed to the logo anchor container */
logoProps?: object;
/** Component to use to wrap the passed <logo> */
logoComponent?: React.ReactNode;
/** Component to render the toolbar (e.g. <Toolbar />) */
toolbar?: React.ReactNode;
/** Component to render the avatar (e.g. <Avatar /> */
avatar?: React.ReactNode;
/** Component to render navigation within the header (e.g. <Nav /> */
/** Component to render the header tools, use <PageHeaderTools /> */
headerTools?: React.ReactNode;
/** Component to render navigation within the header, use <Nav /> */
topNav?: React.ReactNode;
/** True to show the nav toggle button (toggles side nav) */
showNavToggle?: boolean;
Expand All @@ -42,8 +40,7 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
logo = null as React.ReactNode,
logoProps = null as object,
logoComponent = 'a',
toolbar = null as React.ReactNode,
avatar = null as React.ReactNode,
headerTools = null as React.ReactNode,
topNav = null as React.ReactNode,
isNavOpen = true,
role = undefined as string,
Expand Down Expand Up @@ -89,12 +86,7 @@ export const PageHeader: React.FunctionComponent<PageHeaderProps> = ({
pf-c-context-selector
</div> */}
{topNav && <div className={css(styles.pageHeaderNav)}>{topNav}</div>}
{(toolbar || avatar) && (
<div className={css(styles.pageHeaderTools)}>
{toolbar}
{avatar}
</div>
)}
{headerTools}
</header>
);
}}
Expand Down
27 changes: 27 additions & 0 deletions packages/react-core/src/components/Page/PageHeaderTools.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/Page/page';
import { css } from '@patternfly/react-styles';

export interface PageHeaderToolsBreakpointMod {
/** The attribute to modify */
modifier: 'hidden' | 'visible';
/** The breakpoint at which to apply the modifier */
breakpoint?: 'sm' | 'md' | 'lg' | 'xl' | '2xl';
}

export interface PageHeaderToolsProps extends React.HTMLProps<HTMLDivElement> {
/** Content rendered in page header tools */
children: React.ReactNode;
/** Additional classes added to the page header tools. */
className?: string;
}

export const PageHeaderTools: React.FunctionComponent<PageHeaderToolsProps> = ({
children,
className,
...props
}: PageHeaderToolsProps) => (
<div className={css(styles.pageHeaderTools, className)} {...props}>
{children}
</div>
);
32 changes: 32 additions & 0 deletions packages/react-core/src/components/Page/PageHeaderToolsGroup.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/Page/page';
import { css } from '@patternfly/react-styles';
import { PageHeaderToolsBreakpointMod } from './PageHeaderTools';
import { formatBreakpointMods } from '../../helpers/util';

export interface PageHeaderToolsGroupProps extends React.HTMLProps<HTMLDivElement> {
/** Content rendered in the page header tools group */
children: React.ReactNode;
/** Additional classes added to the page header tools group. */
className?: string;
/** An array of breakpoint modifiers to control visibility, e.g. breakpointMods={[{ modifier: 'hidden' }, { modifier: 'visible', breakpoint: 'md' }]} */
breakpointMods?: PageHeaderToolsBreakpointMod[];
}

export const PageHeaderToolsGroup: React.FunctionComponent<PageHeaderToolsGroupProps> = ({
children,
className,
breakpointMods,
...props
}: PageHeaderToolsGroupProps) => (
<div
className={css(
styles.pageHeaderToolsGroup,
breakpointMods && formatBreakpointMods(breakpointMods, styles),
Copy link
Contributor

Choose a reason for hiding this comment

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

I opened an issue agains the page header. formatBreakpointMods function does not work for the 2xl breakpoint.

className
)}
{...props}
>
{children}
</div>
);
34 changes: 34 additions & 0 deletions packages/react-core/src/components/Page/PageHeaderToolsItem.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import * as React from 'react';
import styles from '@patternfly/react-styles/css/components/Page/page';
import { css } from '@patternfly/react-styles';
import { PageHeaderToolsBreakpointMod } from './PageHeaderTools';
import { formatBreakpointMods } from '../../helpers/util';

export interface PageHeaderToolsItemProps extends React.HTMLProps<HTMLDivElement> {
/** Content rendered in page header tools item. */
children: React.ReactNode;
/** Additional classes added to the page header tools item. */
className?: string;
/** An array of breakpoint modifiers to control visibility, e.g. breakpointMods={[{ modifier: 'hidden' }, { modifier: 'visible', breakpoint: 'md' }]} */
breakpointMods?: PageHeaderToolsBreakpointMod[];
/** True to make an icon button appear selected */
isSelected?: boolean;
}

export const PageHeaderToolsItem: React.FunctionComponent<PageHeaderToolsItemProps> = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious how this component works. I think it would be more consistent if we had a .pf-c-page__header-tools-item class that you could apply hidden/visible to. I'm assuming that would still allow us to apply .pf-m-selected to a <Button> used in .pf-c-page__header-tools-item if it has the isSelected prop?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So the problem is, in HTML you can just add the pf-m-selected class to a button anywhere. If you wanted to do this in react you'd have to manually add this class name yourself i.e.

<PageHeaderTools>
  <PageHeaderToolsGroup>
      <Button className="pf-m-selected">
          <CogIcon />
      </Button>
  </PageHeaderToolsGroup>
</PageHeaderTools>

because the selected modifier is not a prop we have available for Buttons, the class is defined in the context of page.

Wrapping the <Button> component in <PageHeaderToolsItem> basically allows me to pass the pf-m-selected class or any other potential modifier down to the child component.

children,
className,
breakpointMods,
isSelected
}: PageHeaderToolsItemProps) => (
<div
className={css(
styles.pageHeaderToolsItem,
isSelected && styles.modifiers.selected,
breakpointMods && formatBreakpointMods(breakpointMods, styles),
className
)}
>
{children}
</div>
);
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@ it('PageHeader should match snapshot (auto-generated)', () => {
logo={null}
logoProps={null}
logoComponent={'a'}
toolbar={null}
avatar={null}
headerTools={null}
topNav={null}
showNavToggle={false}
isNavOpen={true}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/**
* This test was generated
*/
import * as React from 'react';
import { shallow } from 'enzyme';
import { PageHeaderTools } from '../../PageHeaderTools';
// any missing imports can usually be resolved by adding them here
import {} from '../..';

it('PageHeaderTools should match snapshot (auto-generated)', () => {
const view = shallow(<PageHeaderTools children={<div>ReactNode</div>} className={'string'} />);
expect(view).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* This test was generated
*/
import * as React from 'react';
import { shallow } from 'enzyme';
import { PageHeaderToolsGroup } from '../../PageHeaderToolsGroup';
// any missing imports can usually be resolved by adding them here
import {} from '../..';

it('PageHeaderToolsGroup should match snapshot (auto-generated)', () => {
const view = shallow(
<PageHeaderToolsGroup children={<div>ReactNode</div>} className={'string'} breakpointMods={[]} />
);
expect(view).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
/**
* This test was generated
*/
import * as React from 'react';
import { shallow } from 'enzyme';
import { PageHeaderToolsItem } from '../../PageHeaderToolsItem';
// any missing imports can usually be resolved by adding them here
import {} from '../..';

it('PageHeaderToolsItem should match snapshot (auto-generated)', () => {
const view = shallow(
<PageHeaderToolsItem children={<div>ReactNode</div>} className={'string'} breakpointMods={[]} isSelected={true} />
);
expect(view).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ it('PageSection should match snapshot (auto-generated)', () => {
type={'default'}
isFilled={true}
hasNoPadding={false}
breakpointMods={[]}
/>
);
expect(view).toMatchSnapshot();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {} from '../..';

it('PageSidebar should match snapshot (auto-generated)', () => {
const view = shallow(
<PageSidebar className={"''"} nav={<div>ReactNode</div>} isManagedSidebar={true} isNavOpen={true} theme={'light'} />
<PageSidebar className={"''"} nav={<div>ReactNode</div>} isManagedSidebar={true} isNavOpen={true} theme={'dark'} />
);
expect(view).toMatchSnapshot();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PageHeaderTools should match snapshot (auto-generated) 1`] = `
<div
className="pf-c-page__header-tools string"
>
<div>
ReactNode
</div>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PageHeaderToolsGroup should match snapshot (auto-generated) 1`] = `
<div
className="pf-c-page__header-tools-group string"
>
<div>
ReactNode
</div>
</div>
`;
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PageHeaderToolsItem should match snapshot (auto-generated) 1`] = `
<div
className="pf-c-page__header-tools-item pf-m-selected string"
>
<div>
ReactNode
</div>
</div>
`;
10 changes: 5 additions & 5 deletions packages/react-core/src/components/Page/__tests__/Page.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ const props = {
};

test('Check page vertical layout example against snapshot', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" onNavToggle={() => undefined} />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" onNavToggle={() => undefined} />;
const Sidebar = <PageSidebar nav="Navigation" isNavOpen />;
const view = mount(
<Page {...props} header={Header} sidebar={Sidebar}>
Expand All @@ -28,7 +28,7 @@ test('Check page vertical layout example against snapshot', () => {
});

test('Check dark page against snapshot', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" onNavToggle={() => undefined} />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" onNavToggle={() => undefined} />;
const Sidebar = <PageSidebar nav="Navigation" isNavOpen theme="dark" />;
const view = mount(
<Page {...props} header={Header} sidebar={Sidebar}>
Expand All @@ -42,7 +42,7 @@ test('Check dark page against snapshot', () => {
});

test('Check page horizontal layout example against snapshot', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" topNav="Navigation" />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" topNav="Navigation" />;
const Sidebar = <PageSidebar isNavOpen />;
const view = mount(
<Page {...props} header={Header} sidebar={Sidebar}>
Expand All @@ -56,7 +56,7 @@ test('Check page horizontal layout example against snapshot', () => {
});

test('Check page to verify breadcrumb is created', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" topNav="Navigation" />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" topNav="Navigation" />;
const Sidebar = <PageSidebar isNavOpen />;
const PageBreadcrumb = () => (
<Breadcrumb>
Expand All @@ -81,7 +81,7 @@ test('Check page to verify breadcrumb is created', () => {
});

test('Check page to verify skip to content points to main content region', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" topNav="Navigation" />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" topNav="Navigation" />;
const Sidebar = <PageSidebar isNavOpen />;
const PageBreadcrumb = (
<Breadcrumb>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { PageHeader } from '../PageHeader';
jest.mock('../Page');

test('Check page vertical layout example against snapshot', () => {
const Header = <PageHeader logo="Logo" toolbar="Toolbar" avatar=" | Avatar" onNavToggle={() => undefined} />;
const Header = <PageHeader logo="Logo" headerTools="PageHeaderTools | Avatar" onNavToggle={() => undefined} />;
const view = mount(Header);
expect(view).toMatchSnapshot();
});
Loading