-
Notifications
You must be signed in to change notification settings - Fork 843
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.
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
Add EuiDelayHide component #412
Merged
Merged
Changes from all commits
Commits
Show all changes
7 commits
Select commit
Hold shift + click to select a range
c2a4de7
Add EuiDelayHide component
sorenlouv 5e15d35
Updated changelog
sorenlouv b5ca7ef
Added docs
sorenlouv e6511d5
Minor tweak
sorenlouv 1261c8b
Fix issue with component calling setState after unmounting
sorenlouv 65d536d
Address feedback
sorenlouv 67b2a23
Increased example duration to 3000
sorenlouv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,6 +4,7 @@ reports/ | |
tmp/ | ||
dist/ | ||
lib/ | ||
.vscode/ | ||
.DS_Store | ||
.eslintcache | ||
.yo-rc.json | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import React, { Component, Fragment } from 'react'; | ||
import { | ||
EuiDelayHide, | ||
EuiFlexItem, | ||
EuiCheckbox, | ||
EuiFormRow, | ||
EuiFieldNumber, | ||
EuiLoadingSpinner | ||
} from '../../../../src/components'; | ||
|
||
export default class extends Component { | ||
state = { | ||
minimumDuration: 3000, | ||
hide: false | ||
}; | ||
|
||
onChangeMinimumDuration = event => { | ||
this.setState({ minimumDuration: parseInt(event.target.value, 10) }); | ||
}; | ||
|
||
onChangeHide = event => { | ||
this.setState({ hide: event.target.checked }); | ||
}; | ||
|
||
render() { | ||
return ( | ||
<Fragment> | ||
<EuiFlexItem> | ||
<EuiFormRow> | ||
<EuiCheckbox | ||
id="dummy-id" | ||
checked={this.state.hide} | ||
onChange={this.onChangeHide} | ||
label="Hide child" | ||
/> | ||
</EuiFormRow> | ||
<EuiFormRow label="Minimum duration"> | ||
<EuiFieldNumber | ||
value={this.state.minimumDuration} | ||
onChange={this.onChangeMinimumDuration} | ||
/> | ||
</EuiFormRow> | ||
|
||
<EuiFormRow label="Child to render"> | ||
<EuiDelayHide | ||
hide={this.state.hide} | ||
minimumDuration={this.state.minimumDuration} | ||
render={() => <EuiLoadingSpinner size="m"/>} | ||
/> | ||
</EuiFormRow> | ||
</EuiFlexItem> | ||
</Fragment> | ||
); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
import React from 'react'; | ||
import DelayHide from './delay_hide'; | ||
import { GuideSectionTypes } from '../../components'; | ||
import { EuiCode, EuiDelayHide } from '../../../../src/components'; | ||
import { renderToHtml } from '../../services'; | ||
|
||
const delayHideSource = require('!!raw-loader!./delay_hide'); | ||
const delayHideHtml = renderToHtml(DelayHide); | ||
|
||
export const DelayHideExample = { | ||
title: 'DelayHide', | ||
sections: [ | ||
{ | ||
title: 'DelayHide', | ||
source: [ | ||
{ | ||
type: GuideSectionTypes.JS, | ||
code: delayHideSource | ||
}, | ||
{ | ||
type: GuideSectionTypes.HTML, | ||
code: delayHideHtml | ||
} | ||
], | ||
text: ( | ||
<p> | ||
<EuiCode>EuiDelayHide</EuiCode> is a component for conditionally toggling | ||
the visibility of a child component. It will ensure that the child is | ||
visible for at least 1000ms (default). This avoids UI glitches that | ||
are common with loading spinners and other elements that are rendered | ||
conditionally and potentially for a short amount of time. | ||
</p> | ||
), | ||
props: { EuiDelayHide }, | ||
demo: <DelayHide /> | ||
} | ||
] | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import { Component } from 'react'; | ||
import PropTypes from 'prop-types'; | ||
|
||
export class EuiDelayHide extends Component { | ||
static propTypes = { | ||
hide: PropTypes.bool, | ||
minimumDuration: PropTypes.number, | ||
render: PropTypes.func.isRequired | ||
}; | ||
|
||
static defaultProps = { | ||
hide: false, | ||
minimumDuration: 1000 | ||
}; | ||
|
||
constructor(props) { | ||
super(props); | ||
|
||
this.state = { | ||
hide: this.props.hide | ||
}; | ||
|
||
this.lastRenderedTime = this.props.hide ? 0 : Date.now(); | ||
} | ||
|
||
getTimeRemaining(minimumDuration) { | ||
const visibleDuration = Date.now() - this.lastRenderedTime; | ||
return minimumDuration - visibleDuration; | ||
} | ||
|
||
componentWillReceiveProps(nextProps) { | ||
clearTimeout(this.timeout); | ||
const timeRemaining = this.getTimeRemaining(nextProps.minimumDuration); | ||
|
||
if (nextProps.hide && timeRemaining > 0) { | ||
this.setStateDelayed(timeRemaining); | ||
} else { | ||
if (this.state.hide && !nextProps.hide) { | ||
this.lastRenderedTime = Date.now(); | ||
} | ||
|
||
this.setState({ hide: nextProps.hide }); | ||
} | ||
} | ||
|
||
setStateDelayed = timeRemaining => { | ||
this.timeout = setTimeout(() => { | ||
this.setState({ hide: true }); | ||
}, timeRemaining); | ||
}; | ||
|
||
componentWillUnmount() { | ||
clearTimeout(this.timeout); | ||
} | ||
|
||
render() { | ||
if (this.state.hide) { | ||
return null; | ||
} | ||
|
||
return this.props.render(); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import React from 'react'; | ||
import { mount } from 'enzyme'; | ||
import { EuiDelayHide } from './index'; | ||
|
||
describe('when EuiDelayHide is visible initially', () => { | ||
let wrapper; | ||
beforeEach(() => { | ||
jest.useFakeTimers(); | ||
wrapper = mount( | ||
<EuiDelayHide | ||
hide={false} | ||
render={() => <div>Hello World</div>} | ||
/> | ||
); | ||
}); | ||
|
||
test('it should be visible initially', async () => { | ||
wrapper.setProps({ hide: true }); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should be visible after 900ms', () => { | ||
wrapper.setProps({ hide: true }); | ||
jest.advanceTimersByTime(900); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should be hidden after 1100ms', () => { | ||
wrapper.setProps({ hide: true }); | ||
jest.advanceTimersByTime(1100); | ||
expect(wrapper.html()).toEqual(null); | ||
}); | ||
|
||
test('it should be visible after 1100ms regardless of prop changes in-between', () => { | ||
wrapper.setProps({ hide: true }); | ||
wrapper.setProps({ hide: false }); | ||
jest.advanceTimersByTime(1100); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should hide immediately after prop change, if it has been displayed for 1100ms', () => { | ||
const currentTime = Date.now(); | ||
jest.advanceTimersByTime(1100); | ||
jest.spyOn(Date, 'now').mockReturnValue(currentTime + 1100); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
|
||
wrapper.setProps({ hide: true }); | ||
expect(wrapper.html()).toEqual(null); | ||
}); | ||
}); | ||
|
||
describe('when EuiDelayHide is hidden initially', () => { | ||
let wrapper; | ||
beforeEach(() => { | ||
jest.useFakeTimers(); | ||
wrapper = mount( | ||
<EuiDelayHide hide={true} render={() => <div>Hello World</div>} /> | ||
); | ||
}); | ||
|
||
test('it should be hidden initially', async () => { | ||
expect(wrapper.html()).toEqual(null); | ||
}); | ||
|
||
test('it should become visible immediately after prop change', async () => { | ||
wrapper.setProps({ hide: false }); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should be visible for at least 1100ms before hiding', async () => { | ||
wrapper.setProps({ hide: false }); | ||
wrapper.setProps({ hide: true }); | ||
jest.advanceTimersByTime(900); | ||
|
||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
|
||
jest.advanceTimersByTime(200); | ||
expect(wrapper.html()).toEqual(null); | ||
}); | ||
}); | ||
|
||
describe('when EuiDelayHide is visible initially and has a minimumDuration of 2000ms ', () => { | ||
let wrapper; | ||
beforeEach(() => { | ||
jest.useFakeTimers(); | ||
wrapper = mount( | ||
<EuiDelayHide | ||
hide={false} | ||
minimumDuration={2000} | ||
render={() => <div>Hello World</div>} | ||
/> | ||
); | ||
wrapper.setProps({ hide: true }); | ||
}); | ||
|
||
test('it should be visible initially', async () => { | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should be visible after 1900ms', () => { | ||
jest.advanceTimersByTime(1900); | ||
expect(wrapper.html()).toEqual('<div>Hello World</div>'); | ||
}); | ||
|
||
test('it should be hidden after 2100ms', () => { | ||
jest.advanceTimersByTime(2100); | ||
expect(wrapper.html()).toEqual(null); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { EuiDelayHide } from './delay_hide'; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be hidden initially? I think it should behave the same as setting
hide
to true, and should apply the delay. If the consumer really doesn't want it visible, they should just not render it at all.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think if the initial value is
hide=true
there is no reason to display it. The whole point of this component is to avoid "UI glitching". If an element is hidden from the start, it won't need the delay.I think this is very different from a component changing visibility from visible to hidden (which is where we want the delay).
It would also add an extra burden on the consumer. As it is now I can render the global loading spinner with a prop from redux (
isLoading
is a reduced value combining all loading states in the entire app):If
EuiDelayHide
was to always render children on init I would have to keep a flag, to avoid rendering the loading spinner initially:I think we should wait a little with this but if it becomes a requested feature we can add a flag, eg
leading
delay (likedlodash.debounce
has it).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good!