frzyc/genshin-optimizer

Tally should update on add/remove, not all events

Open

#2,051 opened on Apr 30, 2024

View on GitHub
 (2 comments) (0 reactions) (1 assignee)TypeScript (270 forks)auto 404
good first issue

Repository metrics

Stars
 (999 stars)
PR merge metrics
 (PR metrics pending)

Description

This is more of a nit to how we do this in GO, I think these gets rerendered every time we change an artifact/relic, but it should rerender every time an artifact/relic is added/removed (since it only display the total).

It make sense to use the DataManagerCallback.reason string, and check against 'remove' | 'new'. Would also make sense to store total as a state instead of useForceUpdate, so when total is set as the same value again, it doesnt trigger a revalue.

Originally posted by @frzyc in https://github.com/frzyc/genshin-optimizer/pull/2025#discussion_r1582361651

Psudocode:

const [tally, setTally] = useState(database.relics.length)
useEffect(()=>databse.relics.followAny((_,reason)=>['remove','new'].includes(reason) && setTally(database.relics.length),[database])

Example implementation https://github.com/frzyc/genshin-optimizer/pull/2061

Contributor guide

Research direction
Replace useForceUpdate with useState for tally, and filter events by reason 'remove' or 'new'.
Tech stack
typescriptreact
Domain
frontend
Issue type
Refactor
DifficultyEstimated implementation difficulty for a new contributor, from 1 for very small changes to 5 for expert-level work.
1
Estimated timeA rough time range for an experienced contributor to investigate, implement, test, and prepare a pull request.
Under 1 hour
Activity statusHow available the issue appears right now: fresh, active, stale, blocked, or waiting on maintainer input.
Active
ClarityHow clearly the issue explains the expected change, acceptance criteria, and next step.
Clear
Prerequisites
ReactTypeScript
Newbie friendlinessA 1-100 score estimating how approachable this issue is for first-time contributors.
70