Skip to content

Commit

Permalink
Fixed Drawer/NavigationDrawer visibilities
Browse files Browse the repository at this point in the history 
My initial fix in #286 was not actually the real solution since it
caused #288

The visibility will now be changed if there is a resize event after the
initial mount OR the defaultVisible is false and prop.visible is false.

This should now close #288
  • Loading branch information
@mlaursen
mlaursen committed Mar 13, 2017
1 parent 89f42de commit 29b39d1
Show file tree
Hide file tree
Showing 4 changed files with 298 additions and 95 deletions.
50 changes: 31 additions & 19 deletions src/js/Drawers/Drawer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,10 +223,10 @@ export default class Drawer extends PureComponent {

/**
* An optional function to call when the visibility of the drawer is changed. The function will
* be called with the new visibility state and an event that triggered the visibility change.
* be called with the new visibility statee.
*
* ```js
* onVisibilityToggle(!currentlyVisible, event);
* onVisibilityToggle(!currentlyVisible);
* ```
*/
onVisibilityToggle: PropTypes.func,
Expand Down Expand Up @@ -350,6 +350,7 @@ export default class Drawer extends PureComponent {
}

const type = getField(props, this.state, 'type');
this._initialFix = true;

if (typeof props.visible === 'undefined') {
this.state.visible = typeof defaultVisible !== 'undefined'
Expand All @@ -372,9 +373,7 @@ export default class Drawer extends PureComponent {

componentDidMount() {
window.addEventListener('resize', this._updateMedia);
if (typeof this.props.visible === 'undefined' && !this.props.defaultVisible) {
this._updateType(this.props);
}
this._updateType(this.props);
}

componentWillReceiveProps(nextProps) {
Expand Down Expand Up @@ -431,25 +430,38 @@ export default class Drawer extends PureComponent {

const state = Drawer.getCurrentMedia(props);
const diffType = getField(props, this.state, 'type') !== state.type;
const diffMedia = state.mobile !== this.state.mobile
|| state.tablet !== this.state.tablet
|| state.desktop !== this.state.desktop;

if (onMediaTypeChange && (diffType ||
['mobile', 'tablet', 'desktop'].filter(key => state[key] !== this.state[key]).length)
) {
if (onMediaTypeChange && (diffType || diffMedia)) {
onMediaTypeChange(state.type, { mobile: state.mobile, tablet: state.tablet, desktop: state.desktop });
}

if (typeof props.type !== 'undefined') {
delete state.type;
}
if (diffType) {
let visible = isPermanent(state.type);
if (this._initialFix) {
if (props.defaultVisible) {
visible = props.defaultVisible;
} else if (props.visible) {
visible = props.visible;
}

const type = getField(props, state, 'type');
const visible = isPermanent(type);
if (onVisibilityToggle && getField(props, this.state, 'visible') !== visible) {
onVisibilityToggle(visible);
this._initialFix = false;
}
const prevVisible = getField(props, this.state, 'visible');

if (onVisibilityToggle && (visible !== prevVisible)) {
onVisibilityToggle(visible);
}

if (typeof props.visible === 'undefined') {
state.visible = visible;
}
}

if (typeof props.visible === 'undefined' && diffType) {
state.visible = visible;
if (typeof props.type !== 'undefined') {
delete state.type;
}

this.setState(state);
Expand Down Expand Up @@ -509,9 +521,9 @@ export default class Drawer extends PureComponent {
}
}

_closeDrawer(e) {
_closeDrawer() {
if (this.props.onVisibilityToggle) {
this.props.onVisibilityToggle(false, e);
this.props.onVisibilityToggle(false);
}

if (typeof this.props.visible === 'undefined') {
Expand Down
154 changes: 128 additions & 26 deletions src/js/Drawers/__tests__/Drawer.js
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
/* eslint-env jest */
/* eslint-disable max-len */
jest.unmock('../Drawer');
jest.unmock('../../Dialogs/Dialog');
jest.unmock('../DrawerTypes');
jest.unmock('../isType');
jest.unmock('../../constants/media');
jest.unmock('../../Dialogs/Dialog');
jest.unmock('../../Papers/Paper');

import React from 'react';
Expand All @@ -21,33 +24,132 @@ describe('Drawer', () => {
expect(drawer.context.renderNode).toBe(renderNode);
});

it('should call the updateType function after initial mount', () => {
const onVisibilityToggle = jest.fn();
renderIntoDocument(
<Drawer
onVisibilityToggle={onVisibilityToggle}
drawerType={Drawer.DrawerTypes.PERSISTENT}
/>
);
describe('updateType', () => {
const MATCH_MEDIA = window.matchMedia;
const matchesMobile = jest.fn(query => ({
matches: query.indexOf(Drawer.defaultProps.mobileMinWidth) !== -1,
}));
const matchesTablet = jest.fn(query => ({
matches: query.indexOf(Drawer.defaultProps.tabletMinWidth) !== -1,
}));
const matchesDesktop = jest.fn(query => ({
matches: query.indexOf('max') === -1
&& query.indexOf(Drawer.defaultProps.desktopMinWidth) !== -1,
}));
afterAll(() => {
window.matchMedia = MATCH_MEDIA;
});

expect(onVisibilityToggle.mock.calls.length).toBe(1);
});
it('should correctly set the default visibility on mobile devices', () => {
const props = {
navItems: [],
mobileType: Drawer.DrawerTypes.TEMPORARY,
tabletType: Drawer.DrawerTypes.PERSISTENT,
desktopType: Drawer.DrawerTypes.FULL_HEIGHT,
onMediaTypeChange: jest.fn(),
onVisibilityToggle: jest.fn(),
};

window.matchMedia = matchesMobile;
const drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(false);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.TEMPORARY);
expect(props.onMediaTypeChange.mock.calls.length).toBe(0);
expect(props.onVisibilityToggle.mock.calls.length).toBe(0);
});

it('should not call the updateType function after mounting if defaultVisible is true', () => {
const onVisibilityToggle = jest.fn();
const onMediaTypeChange = jest.fn();
const drawer = renderIntoDocument(
<Drawer
defaultVisible
onMediaTypeChange={onMediaTypeChange}
onVisibilityToggle={onVisibilityToggle}
drawerType={Drawer.DrawerTypes.PERSISTENT}
/>
);

expect(onVisibilityToggle.mock.calls.length).toBe(0);
expect(onMediaTypeChange.mock.calls.length).toBe(0);
expect(drawer.state.visible).toBe(true);
it('should correctly set the default visibility on tablets', () => {
const props = {
navItems: [],
mobileType: Drawer.DrawerTypes.TEMPORARY,
tabletType: Drawer.DrawerTypes.PERSISTENT,
desktopType: Drawer.DrawerTypes.FULL_HEIGHT,
onMediaTypeChange: jest.fn(),
onVisibilityToggle: jest.fn(),
};

window.matchMedia = matchesTablet;
const drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(false);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.PERSISTENT);
expect(props.onMediaTypeChange.mock.calls.length).toBe(1);
expect(props.onMediaTypeChange).toBeCalledWith(Drawer.DrawerTypes.PERSISTENT, { mobile: false, tablet: true, desktop: false });
expect(props.onVisibilityToggle.mock.calls.length).toBe(0);
});

it('should correctly set the default visibility on desktop', () => {
const props = {
navItems: [],
mobileType: Drawer.DrawerTypes.TEMPORARY,
tabletType: Drawer.DrawerTypes.PERSISTENT,
desktopType: Drawer.DrawerTypes.FULL_HEIGHT,
onMediaTypeChange: jest.fn(),
onVisibilityToggle: jest.fn(),
};

window.matchMedia = matchesDesktop;
const drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(true);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.FULL_HEIGHT);
expect(props.onMediaTypeChange.mock.calls.length).toBe(1);
expect(props.onMediaTypeChange).toBeCalledWith(Drawer.DrawerTypes.FULL_HEIGHT, { mobile: false, tablet: false, desktop: true });
expect(props.onVisibilityToggle.mock.calls.length).toBe(1);
expect(props.onVisibilityToggle).toBeCalledWith(true);
});

it('should not update the visibility to false when the defaultVisible prop is enabled and the drawer type is temporary for any screen size', () => {
const props = {
defaultVisible: true,
navItems: [],
mobileType: Drawer.DrawerTypes.TEMPORARY,
tabletType: Drawer.DrawerTypes.TEMPORARY,
desktopType: Drawer.DrawerTypes.TEMPORARY,
onMediaTypeChange: jest.fn(),
onVisibilityToggle: jest.fn(),
};

window.matchMedia = matchesMobile;
let drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(true);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.TEMPORARY);
expect(props.onMediaTypeChange.mock.calls.length).toBe(0);
expect(props.onVisibilityToggle.mock.calls.length).toBe(0);

window.matchMedia = matchesTablet;
drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(true);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.TEMPORARY);
expect(props.onMediaTypeChange.mock.calls.length).toBe(1);
expect(props.onMediaTypeChange).toBeCalledWith(props.tabletType, { mobile: false, tablet: true, desktop: false });
expect(props.onVisibilityToggle.mock.calls.length).toBe(0);

window.matchMedia = matchesDesktop;
drawer = renderIntoDocument(<Drawer {...props} />);
expect(drawer.state.visible).toBe(true);
expect(drawer.state.type).toBe(Drawer.DrawerTypes.TEMPORARY);
expect(props.onMediaTypeChange.mock.calls.length).toBe(2);
expect(props.onMediaTypeChange).toBeCalledWith(props.desktopType, { mobile: false, tablet: false, desktop: true });
expect(props.onVisibilityToggle.mock.calls.length).toBe(0);
});

it('should correctly update the visibility when the visible prop was defined and there was a media type change with visibility', () => {
const props = {
visible: false,
defaultMedia: 'mobile',
onMediaTypeChange: jest.fn(),
onVisibilityToggle: jest.fn(),
};

window.matchMedia = matchesDesktop;
renderIntoDocument(<Drawer {...props} />);
expect(props.onMediaTypeChange).toBeCalledWith(Drawer.defaultProps.desktopType, { mobile: false, tablet: false, desktop: true });
expect(props.onVisibilityToggle).toBeCalledWith(true);

window.matchMedia = matchesMobile;
renderIntoDocument(<Drawer {...props} visible defaultMedia="desktop" />);
expect(props.onMediaTypeChange).toBeCalledWith(Drawer.defaultProps.mobileType, { mobile: true, tablet: false, desktop: false });
expect(props.onVisibilityToggle).toBeCalledWith(true);
});
});

describe('matchesMedia', () => {
Expand Down
22 changes: 7 additions & 15 deletions src/js/NavigationDrawers/NavigationDrawer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,10 @@ export default class NavigationDrawer extends PureComponent {

/**
* An optional function to call when the visibility of the drawer changes. The callback
* will include the new visibility and the event that triggered the change.
* will include the new visibility.
*
* ```js
* this.props.onVisibilityToggle(false, event);
* this.props.onVisibilityToggle(false);
* ```
*/
onVisibilityToggle: PropTypes.func,
Expand Down Expand Up @@ -728,9 +728,9 @@ export default class NavigationDrawer extends PureComponent {
}
}

_handleVisibility(visible, e) {
_handleVisibility(visible) {
if (this.props.onVisibilityToggle) {
this.props.onVisibilityToggle(visible, e);
this.props.onVisibilityToggle(visible);
}

if (typeof this.props.visible === 'undefined') {
Expand All @@ -740,24 +740,16 @@ export default class NavigationDrawer extends PureComponent {

_handleTypeChange(drawerType, mediaState) {
const { onMediaTypeChange } = this.props;
let state = mediaState;
if (onMediaTypeChange) {
onMediaTypeChange(drawerType, mediaState);
}

if (typeof this.props.drawerType === 'undefined') {
mediaState.drawerType = drawerType;
state = { ...mediaState, drawerType };
}

const { mobile, tablet, desktop } = this.state;
if (typeof this.props.visible === 'undefined'
&& (mediaState.mobile !== mobile
|| mediaState.tablet !== tablet
|| mediaState.desktop !== desktop)
) {
mediaState.visible = isPermanent(drawerType);
}

this.setState(mediaState);
this.setState(state);
}

render() {
Expand Down
Loading

0 comments on commit 29b39d1

Please sign in to comment.