implement filtering by completion status #180
Conversation
There was a problem hiding this comment.
Love the work you've done here @leo-step! I was able to test the patch out locally and everything is working smoothly. I do have one change request as well as 2 comments/questions for ya!
Let me know if anything doesn't make sense! We're very close to being able to land this 😁
| localStorage.setItem(id, e.target.value); | ||
| setFilter(e.target.value || ''); |
There was a problem hiding this comment.
For my own learning, is this a best practice of updating localStorage first and then state? 🤔
There was a problem hiding this comment.
I swapped the order because the setFilter function for the checkbox filter reads localStorage and needs the value to be up to date. It doesn't make a difference for any of the other filters.
There was a problem hiding this comment.
This makes total sense to me - I forgot we had included this behaviour in #167!
| @@ -31,20 +32,19 @@ import PatternFrequencies from '../PatternFrequencies'; | |||
| const iconPath = `${process.env.PUBLIC_URL}/assets/icons/`; | |||
|
|
|||
| const Table = () => { | |||
| const data = React.useMemo(() => questions, []); | |||
There was a problem hiding this comment.
If I remember correctly, we introduced useMemo() to capture performance gains - see the docs here:
Pass a “create” function and an array of dependencies. useMemo will only recompute the memoized value when one of the dependencies has changed. This optimization helps to avoid expensive calculations on every render.There was a problem hiding this comment.
This new filter is a little different from the other ones because it works with a dynamic checkbox field instead of static properties like difficulty. Previously, the caching worked well because nothing could really be overwritten, but now I encountered problems with it because my assigned properties would constantly be reset. Using useState instead of useMemo solved this problem.
Also I'm not sure if there is a performance gain from useMemo for this line in particular. Isn't it just saving a reference to the questions array? It doesn't seem like a very expensive operation, but correct me if I'm wrong.
Resolves #156