This repository has been archived by the owner on Dec 30, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 386
feat: loading indicator #544
Merged
Merged
Changes from 25 commits
Commits
Show all changes
36 commits
Select commit
Hold shift + click to select a range
12edcd6
chore(package.json): update yarn requirement
bobylito 317e2a5
feat(loading-indicator): add information about stalled searches
bobylito cc40543
feat(loading-indicator): Make timeout for stalled search configurable
bobylito 05eebf0
feat(loading-indicator): add option to searchbox
bobylito 37d353c
chore(ci): update yarn version in travis config
bobylito eb55566
chore: rename stalledSearchTimeout into stalledSearchDelay
bobylito 549368f
fix: revert behaviour change of `searching`
bobylito 499525d
feat(SearchBox): new API for the loading indicator
bobylito b6c6e5b
Merge remote-tracking branch 'origin' into feat/searchbox-loading-ind…
bobylito f2b3ccf
fix: actually add the loading indicator
bobylito 6a3d96b
chore(stories): add with custom loading indicator
bobylito dd9a214
chore(doc): add doc for new API
bobylito 38990cd
chore(createInstantSearchManager): test loading status
bobylito 17f3a3e
chore: remove loading indicator DOM when not needed
bobylito 5dbcaed
Merge branch 'master' into feat/searchbox-loading-indicator
bobylito 090b338
refactor(createInstantSearchManager): isSearchStalled should be in store
bobylito 249a66e
refactoring(InstantSearch): stalledSearchDelay as a defaultProps
bobylito 06c25f8
refactor(SearchBox): isSearchStalled computed once
bobylito 94b5d3d
chore: eslint:fix
bobylito b0c59a4
refactor(SearchBox): use default props for components
bobylito 10e94d1
chore: isSearchStalled should have a default value
bobylito cf84f01
chore: update yarn version (consistency)
bobylito a6783a5
chore: remove props since it's the same value as default
bobylito 4f1e07f
Merge branch 'feat/searchbox-loading-indicator' of github.com:algolia…
bobylito 4999728
Merge branch 'master' into feat/searchbox-loading-indicator
bobylito 7c63e77
chore(storybook): loading indicator as knob
bobylito 5d95fd4
chore: udpate the behaviour and improve test
bobylito 7432ec9
Merge branch 'feat/searchbox-loading-indicator' of github.com:algolia…
bobylito 624a58a
chore: fix lint
bobylito 54ac58c
refactor: variable renaming in createConnector
bobylito a14ed1a
Merge branch 'master' into feat/searchbox-loading-indicator
bobylito f594a62
chore: syntactic sugar
bobylito af014c3
chore(SearchBox): test behaviour with stalled search
bobylito 8521fa3
Merge branch 'feat/searchbox-loading-indicator' of github.com:algolia…
bobylito 84a4614
refactor: consolidate destructuring of props
bobylito c9e156c
Merge branch 'master' into feat/searchbox-loading-indicator
samouss 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
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 |
---|---|---|
|
@@ -133,7 +133,7 @@ | |
}, | ||
"engines": { | ||
"node": "8.6.0", | ||
"yarn": "1.2.0" | ||
"yarn": "1.3.2" | ||
}, | ||
"jest": { | ||
"notify": false, | ||
|
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 |
---|---|---|
|
@@ -5,12 +5,51 @@ import classNames from './classNames.js'; | |
|
||
const cx = classNames('SearchBox'); | ||
|
||
const DefaultLoadingIndicator = () => ( | ||
<svg | ||
width="18" | ||
height="18" | ||
viewBox="0 0 38 38" | ||
xmlns="http://www.w3.org/2000/svg" | ||
stroke="#BFC7D8" | ||
> | ||
<g fill="none" fillRule="evenodd"> | ||
<g transform="translate(1 1)" strokeWidth="2"> | ||
<circle strokeOpacity=".5" cx="18" cy="18" r="18" /> | ||
<path d="M36 18c0-9.94-8.06-18-18-18"> | ||
<animateTransform | ||
attributeName="transform" | ||
type="rotate" | ||
from="0 18 18" | ||
to="360 18 18" | ||
dur="1s" | ||
repeatCount="indefinite" | ||
/> | ||
</path> | ||
</g> | ||
</g> | ||
</svg> | ||
); | ||
|
||
const DefaultReset = () => ( | ||
<svg role="img" width="1em" height="1em"> | ||
<use xlinkHref="#sbx-icon-clear-3" /> | ||
</svg> | ||
); | ||
|
||
const DefaultSubmit = () => ( | ||
<svg role="img" width="1em" height="1em"> | ||
<use xlinkHref="#sbx-icon-search-13" /> | ||
</svg> | ||
); | ||
|
||
class SearchBox extends Component { | ||
static propTypes = { | ||
currentRefinement: PropTypes.string, | ||
refine: PropTypes.func.isRequired, | ||
translate: PropTypes.func.isRequired, | ||
|
||
loadingIndicatorComponent: PropTypes.element, | ||
resetComponent: PropTypes.element, | ||
submitComponent: PropTypes.element, | ||
|
||
|
@@ -25,6 +64,9 @@ class SearchBox extends Component { | |
onReset: PropTypes.func, | ||
onChange: PropTypes.func, | ||
|
||
isSearchStalled: PropTypes.bool, | ||
showLoadingIndicator: PropTypes.bool, | ||
|
||
// For testing purposes | ||
__inputRef: PropTypes.func, | ||
}; | ||
|
@@ -34,6 +76,11 @@ class SearchBox extends Component { | |
focusShortcuts: ['s', '/'], | ||
autoFocus: false, | ||
searchAsYouType: true, | ||
showLoadingIndicator: false, | ||
isSearchStalled: false, | ||
loadingIndicatorComponent: <DefaultLoadingIndicator />, | ||
submitComponent: <DefaultSubmit />, | ||
resetComponent: <DefaultReset />, | ||
}; | ||
|
||
constructor(props) { | ||
|
@@ -156,21 +203,9 @@ class SearchBox extends Component { | |
const { translate, autoFocus } = this.props; | ||
const query = this.getQuery(); | ||
|
||
const submitComponent = this.props.submitComponent ? ( | ||
this.props.submitComponent | ||
) : ( | ||
<svg role="img" width="1em" height="1em"> | ||
<use xlinkHref="#sbx-icon-search-13" /> | ||
</svg> | ||
); | ||
|
||
const resetComponent = this.props.resetComponent ? ( | ||
this.props.resetComponent | ||
) : ( | ||
<svg role="img" width="1em" height="1em"> | ||
<use xlinkHref="#sbx-icon-clear-3" /> | ||
</svg> | ||
); | ||
const submitComponent = this.props.submitComponent; | ||
const resetComponent = this.props.resetComponent; | ||
const loadingIndicatorComponent = this.props.loadingIndicatorComponent; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could use destructuring assignment for those variables. |
||
|
||
const searchInputEvents = Object.keys(this.props).reduce((props, prop) => { | ||
if ( | ||
|
@@ -184,6 +219,9 @@ class SearchBox extends Component { | |
return props; | ||
}, {}); | ||
|
||
const isSearchStalled = | ||
this.props.showLoadingIndicator && this.props.isSearchStalled; | ||
|
||
/* eslint-disable max-len */ | ||
return ( | ||
<form | ||
|
@@ -216,7 +254,10 @@ class SearchBox extends Component { | |
/> | ||
</symbol> | ||
</svg> | ||
<div role="search" {...cx('wrapper')}> | ||
<div | ||
role="search" | ||
{...cx('wrapper', isSearchStalled && 'stalled-search')} | ||
> | ||
<input | ||
ref={this.onInputMount} | ||
type="search" | ||
|
@@ -233,9 +274,22 @@ class SearchBox extends Component { | |
{...searchInputEvents} | ||
{...cx('input')} | ||
/> | ||
{this.props.showLoadingIndicator && ( | ||
<div | ||
style={{ | ||
display: isSearchStalled ? 'block' : 'none', | ||
}} | ||
{...cx('loading-indicator')} | ||
> | ||
{loadingIndicatorComponent} | ||
</div> | ||
)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could add a test for this logic. Verify depending on the given props that the loading component is displayed or not (with a Snapshot). |
||
<button | ||
type="submit" | ||
title={translate('submitTitle')} | ||
style={{ | ||
display: isSearchStalled ? 'none' : 'block', | ||
}} | ||
{...cx('submit')} | ||
> | ||
{submitComponent} | ||
|
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
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
Oops, something went wrong.
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.
We could set this value required since
isSearchStalled
will be always pass to the component.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.
From a usage perspective, I would argue that this value is only useful if
showLoadingIndicator
is true and the only value that we really about istrue
. Considering that, do you think it should still be marked as required?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.
We can leave it as it it then, but when we define an optional props it's recommended to always give them a default value. So we can put a default value to
false
on theisSearchStalled
. WDYT?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.
Ok WFM 👍