Skip to content

Commit e4479aa

Browse files
TheSharpieOneTheSharpieOne
committed
fix(Dropdown): fix perf issue
Now when menu is not 'open' it will not be a popper Closes #584
1 parent d0c6e82 commit e4479aa

4 files changed

Lines changed: 97 additions & 16 deletions

File tree

‎src/DropdownMenu.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ const noFlipModifier = { flip: { enabled: false } };
2727

2828
const DropdownMenu = (props, context) => {
2929
const { className, cssModule, right, tag, flip, ...attrs } = props;
30-
const position1 = context.dropup ? 'top' : 'bottom';
31-
const position2 = right ? 'end' : 'start';
3230
const classes = mapToCssModules(classNames(
3331
className,
3432
'dropdown-menu',
@@ -38,17 +36,24 @@ const DropdownMenu = (props, context) => {
3836
}
3937
), cssModule);
4038

41-
attrs.placement = `${position1}-${position2}`;
39+
let Tag = tag;
40+
41+
if (context.isOpen) {
42+
Tag = Popper;
43+
const position1 = context.dropup ? 'top' : 'bottom';
44+
const position2 = right ? 'end' : 'start';
45+
attrs.placement = `${position1}-${position2}`;
46+
attrs.component = tag;
47+
attrs.modifiers = !flip ? noFlipModifier : undefined;
48+
}
4249

4350
return (
44-
<Popper
51+
<Tag
4552
tabIndex="-1"
4653
role="menu"
4754
{...attrs}
48-
component={tag}
4955
aria-hidden={!context.isOpen}
5056
className={classes}
51-
modifiers={!flip ? noFlipModifier : undefined}
5257
/>
5358
);
5459
};

‎src/DropdownToggle.js‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,15 +13,13 @@ const propTypes = {
1313
cssModule: PropTypes.object,
1414
disabled: PropTypes.bool,
1515
onClick: PropTypes.func,
16-
'data-toggle': PropTypes.string,
1716
'aria-haspopup': PropTypes.bool,
1817
split: PropTypes.bool,
1918
tag: PropTypes.oneOfType([PropTypes.func, PropTypes.string]),
2019
nav: PropTypes.bool,
2120
};
2221

2322
const defaultProps = {
24-
'data-toggle': 'dropdown',
2523
'aria-haspopup': true,
2624
color: 'secondary',
2725
};
@@ -56,7 +54,7 @@ class DropdownToggle extends React.Component {
5654
}
5755

5856
render() {
59-
const { className, color, cssModule, caret, split, nav, tag, 'aria-haspopup': ariaHaspopup, ...props } = this.props;
57+
const { className, color, cssModule, caret, split, nav, tag, ...props } = this.props;
6058
const ariaLabel = props['aria-label'] || 'Toggle Dropdown';
6159
const classes = mapToCssModules(classNames(
6260
className,
@@ -87,7 +85,6 @@ class DropdownToggle extends React.Component {
8785
className={classes}
8886
component={Tag}
8987
onClick={this.onClick}
90-
aria-haspopup={ariaHaspopup ? 'true' : 'false'}
9188
aria-expanded={this.context.isOpen}
9289
children={children}
9390
/>

‎src/__tests__/DropdownMenu.spec.js‎

Lines changed: 81 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from 'react';
2-
import { mount } from 'enzyme';
2+
import { mount, shallow } from 'enzyme';
3+
import { Popper } from 'react-popper';
34
import { DropdownMenu } from '../';
45

56
describe('DropdownMenu', () => {
@@ -63,6 +64,19 @@ describe('DropdownMenu', () => {
6364
expect(wrapper.find('.show').length).toBe(1);
6465
});
6566

67+
it('should render left aligned menus by default', () => {
68+
isOpen = true;
69+
const wrapper = mount(
70+
<DropdownMenu>Ello world</DropdownMenu>,
71+
{
72+
context: { isOpen, dropup, popperManager },
73+
childContextTypes: { popperManager }
74+
}
75+
);
76+
77+
expect(wrapper.find('.dropdown-menu').hasClass('dropdown-menu-right')).toBe(false);
78+
});
79+
6680
it('should render right aligned menus', () => {
6781
isOpen = true;
6882
const wrapper = mount(
@@ -76,6 +90,72 @@ describe('DropdownMenu', () => {
7690
expect(wrapper.find('.dropdown-menu').hasClass('dropdown-menu-right')).toBe(true);
7791
});
7892

93+
it('should render down when dropup is false on the context', () => {
94+
isOpen = true;
95+
const wrapper = shallow(
96+
<DropdownMenu>Ello world</DropdownMenu>,
97+
{
98+
context: { isOpen, dropup, popperManager },
99+
childContextTypes: { popperManager }
100+
}
101+
);
102+
103+
expect(wrapper.find(Popper).prop('placement')).toBe('bottom-start');
104+
});
105+
106+
it('should render up when dropup is true on the context', () => {
107+
isOpen = true;
108+
dropup = true;
109+
const wrapper = shallow(
110+
<DropdownMenu>Ello world</DropdownMenu>,
111+
{
112+
context: { isOpen, dropup, popperManager },
113+
childContextTypes: { popperManager }
114+
}
115+
);
116+
117+
expect(wrapper.find(Popper).prop('placement')).toBe('top-start');
118+
});
119+
120+
it('should render down when dropup is false on the context', () => {
121+
isOpen = true;
122+
const wrapper = shallow(
123+
<DropdownMenu>Ello world</DropdownMenu>,
124+
{
125+
context: { isOpen, dropup, popperManager },
126+
childContextTypes: { popperManager }
127+
}
128+
);
129+
130+
expect(wrapper.find(Popper).prop('placement')).toBe('bottom-start');
131+
});
132+
133+
it('should not disable flip modifier by default', () => {
134+
isOpen = true;
135+
const wrapper = shallow(
136+
<DropdownMenu>Ello world</DropdownMenu>,
137+
{
138+
context: { isOpen, dropup, popperManager },
139+
childContextTypes: { popperManager }
140+
}
141+
);
142+
143+
expect(wrapper.find(Popper).prop('modifiers').flip).toBe(undefined);
144+
});
145+
146+
it('should disable flip modifier when flip is false', () => {
147+
isOpen = true;
148+
const wrapper = shallow(
149+
<DropdownMenu flip={false}>Ello world</DropdownMenu>,
150+
{
151+
context: { isOpen, dropup, popperManager },
152+
childContextTypes: { popperManager }
153+
}
154+
);
155+
156+
expect(wrapper.find(Popper).prop('modifiers')).toEqual({ flip: { enabled: false } });
157+
});
158+
79159
it('should not render multiple children when isOpen is false', () => {
80160
const wrapper = mount(
81161
<DropdownMenu right>Ello world</DropdownMenu>,

‎src/__tests__/DropdownToggle.spec.js‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ describe('DropdownToggle', () => {
2727
);
2828

2929
expect(wrapper.text()).toBe('Ello world');
30-
expect(wrapper.find('[data-toggle="dropdown"]').length).toBe(1);
3130
});
3231

3332
it('should add default sr-only content', () => {
@@ -78,7 +77,7 @@ describe('DropdownToggle', () => {
7877
}
7978
);
8079

81-
expect(wrapper.find('[data-toggle="dropdown"]').hasClass('dropdown-toggle')).toBe(true);
80+
expect(wrapper.hasClass('dropdown-toggle')).toBe(true);
8281
});
8382

8483
describe('color', () => {
@@ -144,7 +143,7 @@ describe('DropdownToggle', () => {
144143
}
145144
);
146145

147-
expect(wrapper.find('[data-toggle="dropdown"]').hasClass('dropdown-toggle-split')).toBe(true);
146+
expect(wrapper.hasClass('dropdown-toggle-split')).toBe(true);
148147
});
149148

150149
describe('onClick', () => {
@@ -215,14 +214,14 @@ describe('DropdownToggle', () => {
215214

216215
it('should not set the tag prop when the tag is defined', () => {
217216
const wrapper = mount(
218-
<DropdownToggle nav tag="span">Ello world</DropdownToggle>,
217+
<DropdownToggle nav>Ello world</DropdownToggle>,
219218
{
220219
context: { isOpen, toggle, popperManager },
221220
childContextTypes: { popperManager }
222221
}
223222
);
224223

225-
expect(wrapper.find('[aria-haspopup="true"]').prop('tag')).toBe(undefined);
224+
expect(wrapper.prop('tag')).toBe(undefined);
226225
});
227226

228227
it('should preventDefault', () => {

0 commit comments

Comments
 (0)