Skip to content

Commit

Permalink
Merge pull request #125 from Gargron/fix-useless-rerenders
Browse files Browse the repository at this point in the history 
Remove function binds in render wherever possible, use PureComponent
  • Loading branch information
@EtienneLem
EtienneLem authored Sep 30, 2017
2 parents 045c776 + 3024f9e commit bbd4fbe
Show file tree
Hide file tree
Showing 6 changed files with 76 additions and 46 deletions.
14 changes: 12 additions & 2 deletions src/components/anchors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'

import SVGs from '../svgs'

export default class Anchors extends React.Component {
export default class Anchors extends React.PureComponent {
constructor(props) {
super(props)

Expand All @@ -14,6 +14,15 @@ export default class Anchors extends React.Component {
this.state = {
selected: defaultCategory.name
}

this.handleClick = this.handleClick.bind(this)
}

handleClick (e) {
var index = e.currentTarget.getAttribute('data-index')
var { categories, onAnchorClick } = this.props

onAnchorClick(categories[index], index)
}

render() {
Expand All @@ -33,7 +42,8 @@ export default class Anchors extends React.Component {
<span
key={name}
title={i18n.categories[name.toLowerCase()]}
onClick={() => onAnchorClick(category, i)}
data-index={i}
onClick={this.handleClick}
className={`emoji-mart-anchor ${isSelected ? 'emoji-mart-anchor-selected' : ''}`}
style={{ color: isSelected ? color : null }}
>
Expand Down
32 changes: 18 additions & 14 deletions src/components/category.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,13 @@ import frequently from '../utils/frequently'
import { Emoji } from '.'

export default class Category extends React.Component {
constructor(props) {
super(props)

this.setContainerRef = this.setContainerRef.bind(this)
this.setLabelRef = this.setLabelRef.bind(this)
}

componentDidMount() {
this.parent = this.container.parentNode

Expand Down Expand Up @@ -133,29 +140,26 @@ export default class Category extends React.Component {
}
}

return <div ref={this.setContainerRef.bind(this)} className={`emoji-mart-category ${emojis && !emojis.length ? 'emoji-mart-no-results' : ''}`} style={containerStyles}>
return <div ref={this.setContainerRef} className={`emoji-mart-category ${emojis && !emojis.length ? 'emoji-mart-no-results' : ''}`} style={containerStyles}>
<div style={labelStyles} data-name={name} className='emoji-mart-category-label'>
<span style={labelSpanStyles} ref={this.setLabelRef.bind(this)}>{i18n.categories[name.toLowerCase()]}</span>
<span style={labelSpanStyles} ref={this.setLabelRef}>{i18n.categories[name.toLowerCase()]}</span>
</div>

{emojis && emojis.map((emoji) =>
Emoji({
emoji: emoji,
...emojiProps
})
Emoji({ emoji: emoji, ...emojiProps })
)}

{emojis && !emojis.length &&
<div>
<div>
{Emoji({
...emojiProps,
size: 38,
emoji: 'sleuth_or_spy',
onOver: null,
onLeave: null,
onClick: null,
})}
{Emoji({
...emojiProps,
size: 38,
emoji: 'sleuth_or_spy',
onOver: null,
onLeave: null,
onClick: null,
})}
</div>

<div className='emoji-mart-no-results-label'>
Expand Down
37 changes: 25 additions & 12 deletions src/components/picker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ const I18N = {
},
}

export default class Picker extends React.Component {
export default class Picker extends React.PureComponent {
constructor(props) {
super(props)

Expand Down Expand Up @@ -115,6 +115,19 @@ export default class Picker extends React.Component {
}

this.categories.unshift(SEARCH_CATEGORY)

this.setAnchorsRef = this.setAnchorsRef.bind(this)
this.handleAnchorClick = this.handleAnchorClick.bind(this)
this.setSearchRef = this.setSearchRef.bind(this)
this.handleSearch = this.handleSearch.bind(this)
this.setScrollRef = this.setScrollRef.bind(this)
this.handleScroll = this.handleScroll.bind(this)
this.handleScrollPaint = this.handleScrollPaint.bind(this)
this.handleEmojiOver = this.handleEmojiOver.bind(this)
this.handleEmojiLeave = this.handleEmojiLeave.bind(this)
this.handleEmojiClick = this.handleEmojiClick.bind(this)
this.setPreviewRef = this.setPreviewRef.bind(this)
this.handleSkinChange = this.handleSkinChange.bind(this)
}

componentWillReceiveProps(props) {
Expand Down Expand Up @@ -206,7 +219,7 @@ export default class Picker extends React.Component {
handleScroll() {
if (!this.waitingForPaint) {
this.waitingForPaint = true
window.requestAnimationFrame(this.handleScrollPaint.bind(this))
window.requestAnimationFrame(this.handleScrollPaint)
}
}

Expand Down Expand Up @@ -368,17 +381,17 @@ export default class Picker extends React.Component {
return <div style={{width: width, ...style}} className='emoji-mart'>
<div className='emoji-mart-bar'>
<Anchors
ref={this.setAnchorsRef.bind(this)}
ref={this.setAnchorsRef}
i18n={this.i18n}
color={color}
categories={this.categories}
onAnchorClick={this.handleAnchorClick.bind(this)}
onAnchorClick={this.handleAnchorClick}
/>
</div>

<Search
ref={this.setSearchRef.bind(this)}
onSearch={this.handleSearch.bind(this)}
ref={this.setSearchRef}
onSearch={this.handleSearch}
i18n={this.i18n}
emojisToShowFilter={emojisToShowFilter}
include={include}
Expand All @@ -387,7 +400,7 @@ export default class Picker extends React.Component {
autoFocus={autoFocus}
/>

<div ref={this.setScrollRef.bind(this)} className='emoji-mart-scroll' onScroll={this.handleScroll.bind(this)}>
<div ref={this.setScrollRef} className='emoji-mart-scroll' onScroll={this.handleScroll}>
{this.getCategories().map((category, i) => {
return <Category
ref={this.setCategoryRef.bind(this, `category-${i}`)}
Expand All @@ -407,17 +420,17 @@ export default class Picker extends React.Component {
sheetSize: sheetSize,
forceSize: native,
backgroundImageFn: backgroundImageFn,
onOver: this.handleEmojiOver.bind(this),
onLeave: this.handleEmojiLeave.bind(this),
onClick: this.handleEmojiClick.bind(this),
onOver: this.handleEmojiOver,
onLeave: this.handleEmojiLeave,
onClick: this.handleEmojiClick,
}}
/>
})}
</div>

{showPreview && <div className='emoji-mart-bar'>
<Preview
ref={this.setPreviewRef.bind(this)}
ref={this.setPreviewRef}
title={title}
emoji={emoji}
emojiProps={{
Expand All @@ -430,7 +443,7 @@ export default class Picker extends React.Component {
}}
skinsProps={{
skin: skin,
onChange: this.handleSkinChange.bind(this)
onChange: this.handleSkinChange
}}
/>
</div>}
Expand Down
13 changes: 3 additions & 10 deletions src/components/preview.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'
import { Emoji, Skins } from '.'
import { getData } from '../utils'

export default class Preview extends React.Component {
export default class Preview extends React.PureComponent {
constructor(props) {
super(props)
this.state = { emoji: null }
Expand All @@ -31,11 +31,7 @@ export default class Preview extends React.Component {

return <div className='emoji-mart-preview'>
<div className='emoji-mart-preview-emoji'>
{Emoji({
key: emoji.id,
emoji: emoji,
...emojiProps,
})}
{Emoji({ key: emoji.id, emoji: emoji, ...emojiProps })}
</div>

<div className='emoji-mart-preview-data'>
Expand All @@ -55,10 +51,7 @@ export default class Preview extends React.Component {
} else {
return <div className='emoji-mart-preview'>
<div className='emoji-mart-preview-emoji'>
{idleEmoji && idleEmoji.length && Emoji({
emoji: idleEmoji,
...emojiProps,
})}
{idleEmoji && idleEmoji.length && Emoji({ emoji: idleEmoji, ...emojiProps })}
</div>

<div className='emoji-mart-preview-data'>
Expand Down
13 changes: 10 additions & 3 deletions src/components/search.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@ import React from 'react'
import PropTypes from 'prop-types'
import emojiIndex from '../utils/emoji-index'

export default class Search extends React.Component {
export default class Search extends React.PureComponent {
constructor (props) {
super(props)

this.setRef = this.setRef.bind(this)
this.handleChange = this.handleChange.bind(this)
}

handleChange() {
var value = this.input.value

Expand All @@ -28,9 +35,9 @@ export default class Search extends React.Component {

return <div className='emoji-mart-search'>
<input
ref={this.setRef.bind(this)}
ref={this.setRef}
type='text'
onChange={this.handleChange.bind(this)}
onChange={this.handleChange}
placeholder={i18n.search}
autoFocus={autoFocus}
/>
Expand Down
13 changes: 8 additions & 5 deletions src/components/skins.js
View file
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
import React from 'react'
import PropTypes from 'prop-types'

export default class Skins extends React.Component {
export default class Skins extends React.PureComponent {
constructor(props) {
super(props)

this.state = {
opened: false,
}

this.handleClick = this.handleClick.bind(this)
}

handleClick(skin) {
handleClick(e) {
var skin = e.currentTarget.getAttribute('data-skin')
var { onChange } = this.props

if (!this.state.opened) {
Expand Down Expand Up @@ -39,9 +42,9 @@ export default class Skins extends React.Component {
className={`emoji-mart-skin-swatch ${selected ? 'emoji-mart-skin-swatch-selected' : ''}`}
>
<span
onClick={() => this.handleClick(skinTone)}
className={`emoji-mart-skin emoji-mart-skin-tone-${skinTone}`}>
</span>
onClick={this.handleClick}
data-skin={skinTone}
className={`emoji-mart-skin emoji-mart-skin-tone-${skinTone}`} />
</span>
)
}
Expand Down

3 comments on commit bbd4fbe

@Gargron
Copy link

@Gargron Gargron commented on bbd4fbe Oct 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dunno what went wrong, but storybook in current master as of this commit is broken. It's complaining about trying to assign to non-object string("kappa"), wherever that came from. I tried nuking my repository to make sure it's not my cache, but that didn't help.

Also, I think because storybook loads components from dist instead of src now live reloading no longer works, which is unfortunate because it means you can't do development with storybook anymore.

@EtienneLem
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmmh 🤔, Storybook is still working on my end. Maybe it’s something in localStorage?
I personally have 2 processes while developing. One for dist (yarn start) and one for storybook (yarn storybook), live reloading works that way 😄

@Gargron
Copy link

@Gargron Gargron commented on bbd4fbe Oct 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you might be right. That would align with me having tested with "kappa" custom emoji in the past. But damn, that's annoying. If we're talking feature requests, it'd be nice to plug my own store backend instead of using localStorage and only being able to customize the namespace. For example, if I could do that, I could save frequently used emojis for the user account and not just the open browser

Please sign in to comment.