[Sboonny]

• I have read Chapter's contributing guidelines.
• My pull request has a descriptive title (not a vague title like Update README.md).
• My pull request targets the main branch of Chapter.

Because of the sort of the data in this PR #2291, I thought maybe a filter function is a due. This should be in Utils later, but for now I am testing it in chapterPages

[ghost]

👇 Click on the image for a new way to code review

• Make big changes easier — review code in small groups of related files

• Know where to start — see the whole change at a glance

• Take a code tour — explore the change with an interactive tour

• Make comments and review — all fully sync’ed with github

  Try it now!

Legend

[Sboonny]

Is this failing because it relies on a condition? The hooks run on mount, but the data is got on load?

[Sboonny]

RIP performance, I guess 🤣. maybe I can move the logic into the top and create a function that run it, but idk if that is actually worth it, while this works

[gikf]

To make it work it would need to be put before if (isLoading || error) return <DashboardLoading error={error} />;, as that condition sometimes can render before the useMemo is used it will make the hook error appear. It might not be worth to for useMemo here at all, as search will be changing at each character anyway.

[gikf]

Works nicely 🚀.

There isn't any issue with complexity, but was there some specific reason for not attempting using useState for the filter string, instead of opting for useReducer?

There's couple nitpicks below.

The search field is not very visually separated from the Add new button. It makes it appear on the first look, like both of them are related.

Moving filter field to it own line might be the quickest way to clearly give them both their own space:

What do you think?

[gikf]

To make it work it would need to be put before if (isLoading || error) return <DashboardLoading error={error} />;, as that condition sometimes can render before the useMemo is used it will make the hook error appear. It might not be worth to for useMemo here at all, as search will be changing at each character anyway.

[Sboonny]

but was there some specific reason for not attempting using useState for the filter string, instead of opting for useReducer?

I had more experience with setState and I wanted to try something else, but after this, I wanted to try to make it with useState, don't know how to apply dispatch to it, tho, maybe with flushSync https://beta.reactjs.org/reference/react-dom/flushSync 🤔.

[gikf]

I had more experience with setState and I wanted to try something else, but after this, I wanted to try to make it with useState, don't know how to apply dispatch to it, tho, maybe with flushSync https://beta.reactjs.org/reference/react-dom/flushSync thinking.

With useState there wouldn't be really what to dispatch. Set function would replace in onChange dispatch from reducer and that's basically it.

[ojeytonwilliams]

I feel bad asking you to replace the reducer, but it's more than we need here.

[Sboonny]

did you know that the type will fail if it's {filterChapter, setFilterChapter}. I have spent a good while on it, trying to use generics, until I gave up and copied hasMore into it.

The failing type is, in hindsight, I should have noticed the array 🤣

[string, Dispatch<setStateAction>]

[ojeytonwilliams]

hasMore? The code you've written seems fine as is.

Requested change was implemented