I Reviewed Your Beginner React Code

As if recruiters would know what a commit is or gave a fuck about the commit conventions.

Type at plural don't always come for arrays, for example component props types. What you find to want to be singular is mostly "entity" based types (like User, Product, etc)

There is only one perfect solution for the mobile part. Put in the effort to make it responsive. This is especially needed when someone could look at your website on a smaller device (not supported seems sloppy for such a small scale application). Navigator.userAgent docs: "Browser identification based on detecting the user agent string is unreliable and is not recommended, as the user agent string is user configurable." With the changes in the video you could also potentially get a bug.

Calling `useSytem` again from within TimeCategory will re-execute all of its code and hooks, creating new instances of all of the system's state/hooks local to TimeCategory. (despite destructuring only setLocalStorage)
This approach would only be valid if useSystem stored everything in a Context or will lead to problems for any props that depend on shared state.

Before we change something. we should know what we are changing. I think.
Example. (technically the reason result have s is because the person coding thinks the interface have a lot of members)
result here is an interface. by convention it should have capital I so shouldn't it be IResult?
IResult is very vague. If you look at the purpose. It is the user result. So maybe it should be IUserResult.

Extra advice or opinion Interface on most cases I think shouldn't be made plural. So no or don't use IUserResults.
Instead make the one referencing it the plural or something.
Ex.
(Singular)
const [userResult, setUserResult] = useState<IUserResult>(...)
(Plural)
const [userResults, setUserResults] = useState<IUserResult[]>(...)
(Another way of making it plural is)
const [userResultLists, setUserResultLists] = useState<IUserResult[]>(...)

1st one is literally just a monkey type clone bruh

I agree with the css change, lots of extra code removed for the actual non-mobile user

well your own code need code review by at least a senior dev

wait, so that's a beginner react code? I feel so down right now knowing that this is a beginner one 😂

I think most of these make sense 👍🏻
If there's another project you review, maybe you could consider adding in a few words about testing. Even if this is a fun project for a resume, the author would probably still like to have (automatic) dependency updates. Having some tests is much better than nothing.

Looks like they wanted to have some tests but the only test file is empty.

Another thing I saw in the code is that the author could use "as const" to define some static configuration, although this is more of a curiosity than an actual useful suggestion in this case.

Nice project!

disliked, I disagree with nearly 80% of what you said. the comments are clear.

Atomic hooks is very important point.

Only 90k subscribers. Really?

the way I see the custom hook for devices is like an excuse he/she made to just keep practicing some logic and react functionality in general, something that the "easier" way with a couple of css lines won't give you as a beginner, the tip you gave is alright I guess, not the best approach either lol, it looks more like a work around to me tbh.

Can't agree with all your improvements. :(

Thank you Josh

My phone counts as large by your CSS implementation. In fact, it probably would count as 2xl. You wouldn't want to use device size to discriminate between mobile and PC. Especially when there is a such a variety of screen sizes between mobile and PC.

React = 💩

Some of my commit messages: \M/, devil, 666, FUN (.❛ ᴗ ❛.), hmm, (σ≧▽≦)σ

style commit prefix its not about ui changes, it means codestyle changes

Josh, I'm fetching on the server with trpc and mutating on the client, how do I revalidate data?

This is very helpful for new beginner like me. Thank you very much!!!

chore: fix

Would you recommend Angular 17 vs next.js?

conditionally render by checking only the device width is too junior to become a review on code

Can anyone tell me any best way for commit

No body gives this in-depth knowledge and tips thanks for that

beautiful video, but I would argue that making the game hide on small screen is bad considering why he did that in the first place.
you're hiding just on small screen which connects to a responsive design, he did that not because a phone is small (you can have tablets larger than your laptop screen), but because the tracking hook he used does not work without a keyboard (e.g. doesn't work on a digital one -mobile/laptop).

so by your approach, someone with a big enough tablet can reach this website and play the game even when he can't.
using mobile agent here is the right approach, but as you said hard coding the values and getting all the agents probably will take him a lot of time and he will some,
for that case he should use a library like `react-device-detect`(769k weekly downloads), which renders a view based on the browser's agent with agent sniffing.

This wasnt a code review of React. This was a public shaming of someone who doesnt follow your specific conventions and style guides.

All these suggestions are “nitpicks” except for maybe the hook restructure.

As long as you commits aren’t “chore: wip” write whatever you want. It literally doesn’t matter. Most teams use PRs and squash so you never see the original commit msg

Width check for disabling stuff could be problematic runs the risk of disabling stuff when it shouldn’t.

The Results type is hold the Result’s just like if you had a Settings type that had language and dark mode etc, you wouldn’t call that type Setting? Also who cares? Find and replace is a thing if it’s meaning changed.

These are all extremely superficial things and worrying about these things is what keeps people as a beginner. Learn how to use hooks properly, how to do safer conditional render. Hell how to actually style things effectively is more important.

With AI starting to write a lot of code know what is bullshit and not is more valuable that how to name things.

I like the improvements to commit messages. That's one of the first things we did on our small team, because everyone's commit messages and titles for PRs were so different. I wonder too, if showing that the project pushed PRs up instead of commiting direclty to main could show a level of profesionalism. Then someone could write more about the changes they were making, even if they were the only person on the project.

I also wonder if the original intention of the one big hook in the main App.tsx file was about setting up a container/presentational component pattern. By adding all the logic into the App.tsx all the child components are presentational, and can be more easily tested, and updated, with limited mocking. I agree though, splitting up the hook makes sense.

🤓🤓🤓🤓

The first one was Monkey Type right? since it is open source.

commitLint and the Excalidraw app is new and very helpful to me. Thank you, Josh!

I edge to your videos

Anyone who cares about commit messages should be fired immediatly

Removing whitespace should fall under style and not refactor since it doesn't necessarily change the meaning of the code.

More of this please!

Make more of these!!

Someone just forked monketype and shared it 🤣🤣

Style conventional commit message is for code style not styling in the UI sense

commitlint conventions were super helpful did not know about that

wow! this was really useful, as someone who's been a back-end developer for so long now trying to get into react, am just learning a lot.

how do i submit my front-end projects for review?

If this code is written by a beginner then who am I 😂?

Git commits often involves different changes in code so you can't specify the exact change.I personally use "1:commit" and so on....

Is that a joke when he did a whole hook to detect mobile version for optimization and you just tell him to hide it with css 😢😢 ?