Skip to content

Commit 8c50170

Browse files
redallenredallen
authored
fix(ToolbarToggleGroup): revert show to breakpoint (#4343)
* fix overflowmenu * revert ToolbarToggleGroup to single breakpoint * add warnings when invisible
1 parent a34647c commit 8c50170

File tree

10 files changed

+73
-46
lines changed

10 files changed

+73
-46
lines changed

‎packages/react-core/src/components/OverflowMenu/OverflowMenu.tsx

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -2,20 +2,26 @@ import * as React from 'react';
22
import styles from '@patternfly/react-styles/css/components/OverflowMenu/overflow-menu';
33
import { css } from '@patternfly/react-styles';
44
import { OverflowMenuContext } from './OverflowMenuContext';
5-
/* eslint-disable camelcase */
6-
import global_breakpoint_md from '@patternfly/react-tokens/dist/js/global_breakpoint_md';
7-
import global_breakpoint_lg from '@patternfly/react-tokens/dist/js/global_breakpoint_lg';
8-
import global_breakpoint_xl from '@patternfly/react-tokens/dist/js/global_breakpoint_xl';
9-
/* eslint-enable camelcase */
5+
import mdBreakpoint from '@patternfly/react-tokens/dist/js/global_breakpoint_md';
6+
import lgBreakpoint from '@patternfly/react-tokens/dist/js/global_breakpoint_lg';
7+
import xlBreakpoint from '@patternfly/react-tokens/dist/js/global_breakpoint_xl';
8+
import xl2Breakpoint from '@patternfly/react-tokens/dist/js/global_breakpoint_2xl';
109
import { debounce } from '../../helpers/util';
1110

11+
const breakpoints = {
12+
md: mdBreakpoint,
13+
lg: lgBreakpoint,
14+
xl: xlBreakpoint,
15+
'2xl': xl2Breakpoint
16+
};
17+
1218
export interface OverflowMenuProps extends React.HTMLProps<HTMLDivElement> {
1319
/** Any elements that can be rendered in the menu */
1420
children?: any;
1521
/** Additional classes added to the OverflowMenu. */
1622
className?: string;
1723
/** Indicates breakpoint at which to switch between horizontal menu and vertical dropdown */
18-
breakpoint: 'md' | 'lg' | 'xl';
24+
breakpoint: 'md' | 'lg' | 'xl' | '2xl';
1925
}
2026

2127
export interface OverflowMenuState extends React.HTMLProps<HTMLDivElement> {
@@ -40,23 +46,21 @@ export class OverflowMenu extends React.Component<OverflowMenuProps, OverflowMen
4046
}
4147

4248
handleResize = () => {
43-
const breakpoints: { [index: string]: { value: string } } = {
44-
/* eslint-disable camelcase */
45-
md: global_breakpoint_md,
46-
lg: global_breakpoint_lg,
47-
xl: global_breakpoint_xl
48-
/* eslint-enable camelcase */
49-
};
50-
const { breakpoint } = this.props;
51-
let breakpointWidth: string | number = breakpoints[breakpoint].value;
52-
breakpointWidth = Number(breakpointWidth.split('px')[0]);
49+
const breakpointPx = breakpoints[this.props.breakpoint];
50+
if (!breakpointPx) {
51+
// eslint-disable-next-line no-console
52+
console.error('OverflowMenu will not be visible without a valid breakpoint.');
53+
return;
54+
}
55+
const breakpointWidth = Number(breakpointPx.value.replace('px', ''));
5356
const isBelowBreakpoint = window.innerWidth < breakpointWidth;
54-
this.state.isBelowBreakpoint !== isBelowBreakpoint && this.setState({ isBelowBreakpoint });
57+
this.setState({ isBelowBreakpoint });
5558
};
5659

5760
render() {
5861
// eslint-disable-next-line @typescript-eslint/no-unused-vars
5962
const { className, breakpoint, children, ...props } = this.props;
63+
6064
return (
6165
<div {...props} className={css(styles.overflowMenu, className)}>
6266
<OverflowMenuContext.Provider value={{ isBelowBreakpoint: this.state.isBelowBreakpoint }}>

‎packages/react-core/src/components/OverflowMenu/__tests__/OverflowMenu.test.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as React from 'react';
2-
import { render, mount } from 'enzyme';
2+
import { render, shallow, mount } from 'enzyme';
33
import styles from '@patternfly/react-styles/css/components/OverflowMenu/overflow-menu';
44
import { OverflowMenu } from '../OverflowMenu';
55

@@ -28,4 +28,15 @@ describe('OverflowMenu', () => {
2828
);
2929
expect(view).toMatchSnapshot();
3030
});
31+
32+
test('should warn on bad props', () => {
33+
const myMock = jest.fn() as any;
34+
global.console = { error: myMock } as any;
35+
shallow(
36+
<OverflowMenu breakpoint={undefined as "md"}>
37+
<div>BASIC</div>
38+
</OverflowMenu>
39+
);
40+
expect(myMock).toBeCalled();
41+
});
3142
});

‎packages/react-core/src/components/Toolbar/ToolbarToggleGroup.tsx

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,13 @@ import { ToolbarContext, ToolbarContentContext } from './ToolbarUtils';
77
import { Button } from '../Button';
88
import globalBreakpointLg from '@patternfly/react-tokens/dist/js/global_breakpoint_lg';
99

10-
import { formatBreakpointMods, toCamel } from '../../helpers/util';
10+
import { formatBreakpointMods, toCamel, capitalize } from '../../helpers/util';
1111

1212
export interface ToolbarToggleGroupProps extends ToolbarGroupProps {
1313
/** An icon to be rendered when the toggle group has collapsed down */
1414
toggleIcon: React.ReactNode;
15+
/** Controls when filters are shown and when the toggle button is hidden. */
16+
breakpoint: 'md' | 'lg' | 'xl' | '2xl';
1517
/** Visibility at various breakpoints. */
1618
visiblity?: {
1719
default?: 'hidden' | 'visible';
@@ -20,14 +22,6 @@ export interface ToolbarToggleGroupProps extends ToolbarGroupProps {
2022
xl?: 'hidden' | 'visible';
2123
'2xl'?: 'hidden' | 'visible';
2224
};
23-
/** Controls when filters are shown and when the toggle button is hidden. */
24-
show?: {
25-
default?: 'show';
26-
md?: 'show';
27-
lg?: 'show';
28-
xl?: 'show';
29-
'2xl'?: 'show';
30-
};
3125
/** Alignment at various breakpoints. */
3226
alignment?: {
3327
default?: 'alignRight' | 'alignLeft';
@@ -66,7 +60,7 @@ export class ToolbarToggleGroup extends React.Component<ToolbarToggleGroupProps>
6660
toggleIcon,
6761
variant,
6862
visiblity,
69-
show,
63+
breakpoint,
7064
alignment,
7165
spacer,
7266
spaceItems,
@@ -75,6 +69,11 @@ export class ToolbarToggleGroup extends React.Component<ToolbarToggleGroupProps>
7569
...props
7670
} = this.props;
7771

72+
if (!breakpoint && !toggleIcon) {
73+
// eslint-disable-next-line no-console
74+
console.error('ToolbarToggleGroup will not be visible without a breakpoint or toggleIcon.');
75+
}
76+
7877
return (
7978
<ToolbarContext.Consumer>
8079
{({ isExpanded, toggleIsExpanded }) => (
@@ -92,13 +91,20 @@ export class ToolbarToggleGroup extends React.Component<ToolbarToggleGroupProps>
9291
<div
9392
className={css(
9493
styles.toolbarGroup,
94+
styles.modifiers.toggleGroup,
9595
variant && styles.modifiers[toCamel(variant) as 'filterGroup' | 'iconButtonGroup' | 'buttonGroup'],
96+
breakpoint &&
97+
styles.modifiers[
98+
`showOn${capitalize(breakpoint.replace('2xl', '_2xl'))}` as
99+
| 'showOnMd'
100+
| 'showOnLg'
101+
| 'showOnXl'
102+
| 'showOn_2xl'
103+
],
96104
formatBreakpointMods(visiblity, styles),
97-
formatBreakpointMods(show, styles),
98105
formatBreakpointMods(alignment, styles),
99106
formatBreakpointMods(spacer, styles),
100107
formatBreakpointMods(spaceItems, styles),
101-
styles.modifiers.toggleGroup,
102108
className
103109
)}
104110
{...props}

‎packages/react-core/src/components/Toolbar/__tests__/Generated/ToolbarToggleGroup.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,6 @@ import { ToolbarToggleGroup } from '../../ToolbarToggleGroup';
88
import {} from '../..';
99

1010
it('ToolbarToggleGroup should match snapshot (auto-generated)', () => {
11-
const view = shallow(<ToolbarToggleGroup toggleIcon={<div>ReactNode</div>} />);
11+
const view = shallow(<ToolbarToggleGroup breakpoint="xl" toggleIcon={<div>ReactNode</div>} />);
1212
expect(view).toMatchSnapshot();
1313
});

‎packages/react-core/src/components/Toolbar/__tests__/ToolbarToggleGroup.test.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import * as React from 'react';
2+
import { shallow } from 'enzyme';
3+
import { ToolbarToggleGroup } from '../ToolbarToggleGroup';
4+
5+
describe('ToolbarToggleGroup', () => {
6+
it('should warn on bad props', () => {
7+
const myMock = jest.fn() as any;
8+
global.console = { error: myMock } as any;
9+
shallow(<ToolbarToggleGroup breakpoint={undefined as 'xl'} toggleIcon={null} />);
10+
expect(myMock).toBeCalled();
11+
});
12+
});

‎packages/react-core/src/components/Toolbar/examples/Toolbar.md

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -402,7 +402,7 @@ class ToolbarComponentMangedToggleGroup extends React.Component {
402402
</ToolbarGroup>
403403
</React.Fragment>;
404404

405-
const items = <ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>;
405+
const items = <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>;
406406

407407
return <Toolbar id="toolbar-component-managed-toggle-groups" className='pf-m-toggle-group-container'>
408408
<ToolbarContent>
@@ -556,7 +556,7 @@ class ToolbarConsumerMangedToggleGroup extends React.Component {
556556
</ToolbarGroup>
557557
</React.Fragment>;
558558

559-
const items = <ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>;
559+
const items = <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>;
560560

561561
return (
562562
<Toolbar id="toolbar-consumer-managed-toggle-groups"
@@ -787,8 +787,7 @@ class ToolbarWithFilterExample extends React.Component {
787787
];
788788

789789
const toolbarItems = <React.Fragment>
790-
<ToolbarToggleGroup toggleIcon={<FilterIcon />}
791-
show={{ xl: 'show' }}>
790+
<ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">
792791
{toggleGroupItems}
793792
</ToolbarToggleGroup>
794793
<ToolbarGroup variant="icon-button-group">
@@ -1016,7 +1015,7 @@ class ToolbarStacked extends React.Component {
10161015

10171016

10181017
const firstRowItems = <React.Fragment>
1019-
<ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>
1018+
<ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>
10201019
<ToolbarGroup variant="icon-button-group">{iconButtonGroupItems}</ToolbarGroup>
10211020
<ToolbarItem variant="overflow-menu">Overflow Menu</ToolbarItem>
10221021
</React.Fragment>;

‎packages/react-core/src/demos/FilterTableDemo.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ class FilterTableDemo extends React.Component {
320320
collapseListedFiltersBreakpoint="xl"
321321
>
322322
<ToolbarContent>
323-
<ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>
323+
<ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">
324324
<ToolbarGroup variant="filter-group">
325325
{this.buildCategoryDropdown()}
326326
{this.buildFilterDropdown()}

‎packages/react-core/src/demos/MasterDetailDemo.md

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ class MasterDetailFullPage extends React.Component {
407407
</ToolbarGroup>
408408
</React.Fragment>;
409409

410-
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>;
410+
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>;
411411

412412
const panelContent = (
413413
<DrawerPanelContent>
@@ -935,7 +935,7 @@ class MasterDetailContentPadding extends React.Component {
935935
</ToolbarGroup>
936936
</React.Fragment>;
937937

938-
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>;
938+
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>;
939939

940940
const panelContent = (
941941
<DrawerPanelContent>
@@ -2447,7 +2447,7 @@ class MasterDetailInlineModifier extends React.Component {
24472447
</ToolbarGroup>
24482448
</React.Fragment>;
24492449

2450-
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }}>{toggleGroupItems}</ToolbarToggleGroup>;
2450+
const ToolbarItems = <ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl">{toggleGroupItems}</ToolbarToggleGroup>;
24512451

24522452
const panelContent = (
24532453
<DrawerPanelContent>

‎packages/react-integration/demo-app-ts/src/Demos.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -165,11 +165,6 @@ export const Demos: DemoInterface[] = [
165165
name: 'Data List Compact Demo',
166166
componentType: Examples.DataListCompactDemo
167167
},
168-
{
169-
id: 'toolbar-demo',
170-
name: 'Toolbar Demo',
171-
componentType: Examples.ToolbarDemo
172-
},
173168
{
174169
id: 'divider-demo',
175170
name: 'Divider Demo',

‎packages/react-integration/demo-app-ts/src/components/demos/ToolbarDemo/ToolbarDemo.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ export class ToolbarDemo extends React.Component<ToolbarProps, ToolbarState> {
247247

248248
const toolbarItems = (
249249
<React.Fragment>
250-
<ToolbarToggleGroup toggleIcon={<FilterIcon />} show={{ xl: 'show' }} id="demo-toggle-group">
250+
<ToolbarToggleGroup toggleIcon={<FilterIcon />} breakpoint="xl" id="demo-toggle-group">
251251
{toggleGroupItems}
252252
</ToolbarToggleGroup>
253253
<ToolbarGroup variant="icon-button-group">

0 commit comments

Comments
 (0)