Hello mate!

[padelis]

Hello @juraj98!

Thanks for taking the time and effort to do this assignment. Nice implementation with trpc 💯 🚀 I'll put some questions below if you can take a look

1. In the first place how did you chose next.js? Did the server part made any significant difference for this project?

2. Have you ever used any frontend testing libraries like jest or react-testing-library?

3. And one last about here:

       onClose={() => {
         window.history.pushState({ href: "/" }, "", "/");
         return setImageData(null);
       }}

   Isn't a bit strange UX to change the URL when closing a modal? wdyt?

[juraj98]

Hello @padelis,

thanks for taking the time review the project. To answer your questions:

1. I chose Next.js for the ability to add OpenGraph data.
   
   The SSR is only used on /image/[imageId] page (You can see it in code here). So when user wants to send a picture of a cat to their friends through social media, the OpenGraph data are present. The result is a preview link that provides more information than it would be possible with a client-side rendered site alone. You can see the preview here.
   
   For all other pages, I'm not blocking the first render by waiting for api requests through SSR, since this would result in worse UX. Instead I'm displaying loading grid indicating that the images are loading.
   
   One more advantage of using Next.js is that since I've created api routes that connect to the Cat API, I'm not exposing the Cat API's API key to the client.

2. Yes, I've used jest, cypress, and react-testing-library in most of my previous jobs. I'll add some tests to this project as well, just to show how I'd go about it.

3. Oops, good catch! This is not an intended behavior.
   
   The URL should be only changed when closing the modal on the Home page. If you click on any image on homepage, the modal is opened and url is changed to /image/[imageId], so when you close the modal, the url should be changed back to /.
   
   At first, I had multiple hooks for each page, but I consolidated them when I switched to trpc and forgot about this.
   
   Anyway, I've fixed it already, so it should work as intended.

[padelis]

Thank you for the prompt replies. 🙇 Wow about 1. Didn't notice that at all