Skip to content
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

useSteps(): only update setStep when needed #774

Merged
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/components/Preferences/LocalIdentities.js
Expand Up @@ -30,6 +30,9 @@ const LocalIdentities = ({
onModify,
onModifyEvent,
}) => {
const { identityEvents$ } = React.useContext(IdentityContext)
const { showLocalIdentityModal } = React.useContext(LocalIdentityModalContext)

// transform localIdentities from object into array and attach address to each entry
const identities = Object.entries(localIdentities).map(
([address, identity]) => ({
Expand All @@ -42,8 +45,6 @@ const LocalIdentities = ({
return <EmptyLocalIdentities onImport={onImport} />
}

const { identityEvents$ } = React.useContext(IdentityContext)
const { showLocalIdentityModal } = React.useContext(LocalIdentityModalContext)
Copy link
Contributor

Choose a reason for hiding this comment

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

I wanted to avoid making this change until after #768 was merged, so we don't have too many conflicts :).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted :+1:

const updateLabel = address => async () => {
try {
await showLocalIdentityModal(address)
Expand Down
59 changes: 42 additions & 17 deletions src/hooks.js
@@ -1,4 +1,4 @@
import { useCallback, useEffect, useState } from 'react'
import { useCallback, useEffect, useReducer, useState } from 'react'
import keycodes from './keycodes'
import { log, removeStartingSlash } from './utils'

Expand Down Expand Up @@ -41,32 +41,57 @@ export function useArrows({ onUp, onLeft, onDown, onRight } = {}) {
}, [onUp, onLeft, onDown, onRight])
}

function stepsReducer(state, { type, value, steps }) {
const { step } = state

let newStep = null

if (type === 'set') {
newStep = value
}
if (type === 'next' && step < steps - 1) {
newStep = step + 1
}
if (type === 'prev' && step > 0) {
newStep = step - 1
}

if (newStep !== null && step !== newStep) {
return {
step: newStep,
direction: newStep > step ? 1 : -1,
}
}

return state
}

// Simple hook to manage a given number of steps.
export function useSteps(steps) {
const [step, _setStep] = useState(0)
const [direction, setDirection] = useState(0)
const [{ step, direction }, updateStep] = useReducer(stepsReducer, {
step: 0,
direction: 0,
})

// If the number of steps change, we reset the current step
useEffect(() => {
updateStep({ type: 'set', value: 0, steps })
}, [steps])

const setStep = useCallback(
newStep => {
if (step !== newStep) {
setDirection(newStep > step ? 1 : -1)
_setStep(newStep)
}
value => {
updateStep({ type: 'set', value, steps })
},
[setDirection, _setStep, step]
[steps]
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we not need to include the reducer's updateStep because it'll never change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly, setSomething from useState() and dispatch from useReducer() are both guaranteed to never update [1], and don’t need to be added to the dependencies list.

[1] Search for “guarantees” in these two sections:

https://reactjs.org/docs/hooks-reference.html#usestate
https://reactjs.org/docs/hooks-reference.html#usereducer

)

const next = useCallback(() => {
if (step < steps - 1) {
setStep(step + 1)
}
}, [setStep, step, steps])
updateStep({ type: 'next', steps })
}, [steps])

const prev = useCallback(() => {
if (step > 0) {
setStep(step - 1)
}
}, [setStep, step])
updateStep({ type: 'prev', steps })
}, [steps])

return {
direction,
Expand Down