Skip to content

Feat/implement cats gallery app#33

Open
AlexNti wants to merge 19 commits into
GlobalWebIndex:mainfrom
AlexNti:feat/implement-cats-gallery-app
Open

Feat/implement cats gallery app#33
AlexNti wants to merge 19 commits into
GlobalWebIndex:mainfrom
AlexNti:feat/implement-cats-gallery-app

Conversation

@AlexNti
Copy link
Copy Markdown

@AlexNti AlexNti commented Jul 10, 2025

🚀 What’s New
This PR adds three new pages to the cats application:

  • Cats Gallery: Displays a list of cat images with pagination. Users can load more images as they scroll and view details or favorite an image by clicking on it.

  • Breeds Page: Shows a list of all available cat breeds. Clicking on a breed shows related images. Selecting an image opens a modal with breed details.

  • Favourites Page: Displays all the user’s favorite images. Users can remove favorites from this page.

❗ Please read the README.md for setup instructions, design decisions, architecture details, and general project information.

AlexNti added 9 commits July 10, 2025 12:07
Set up the initial project using Next.js as the React framework and Tailwind CSS for styling.
Added the custom styling theme based on the neo-brutalist design.
This commit introduces the Cat list view, where we fetch and display 10 cats at a time. Users can load more cats when reaching the end of the list.

Additionally, we’ve introduced several reusable UI components for broader use across the application:

- Button
- Alert
- ServerSideError
This commit introduces a new modal that displays detailed information about a cat when its image is clicked. If breed data is available, it is shown alongside the image.

Users can also now mark images as favorite or unfavorite.

New hooks and components added:

* useDebounce hook

* Modal component

* IconButton component

* Star rating component
This commit introduces a new breeds page that displays a list of available cat breeds. Users can:
- Click on a breed to open a modal showing images of that breed.
- Click on any image to navigate to the image detail modal, which includes a larger version of the image and breed information.
This commit introduces a new favourites page, allowing users to view their favourite cat images and unfavourite them if desired.

To support this feature, we implemented persistent user identification:
- When a new user opens the browser, a unique userId is generated.
- The ID is stored in a cookie (for server-side access) and in localStorage (for easier access on the client).
…nd error handling

* Rename favorite => favourite
* Improve modal rendering strategy with parallel routes of nextjs
* Enabled Next.js cache to improve performance
* Replaced inline alert handling with a reusable alert hook
* Made useFetch more scalable by allowing custom props to be passed to the execute function
* Improved error handling across the app
* Added skeleton loaders throughout the app for a better loading experience
* Now the cat card is immediately removed after unfavourite
* Move fetching of favourites on the server side
In this commit we are improving the docs on how to setup the env variables for the app, we also include a script to make it automatically.
We also add some clarification on the docs regarding the tests
cursor[bot]

This comment was marked as outdated.

This commit addresses an issue where the uuid on client side could be different on server side and on client side.
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

AlexNti added 2 commits July 10, 2025 21:39
In this commit we address two comments of of cursor:

- We simplify the "authentication" of the user by setting the userID on the middleware, this will guarantee that
the user id will always be there and we will not need to fallback to default value.
- We enhance the condition regarding if we call or not the delete cat from favourite
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

In this commit we are creating a new server function to favourite/unfavourite an image,
by moving the logic in server function is easier to handle the response in the client,\
making a more simple implementation.
cursor[bot]

This comment was marked as outdated.

We are making this change to address a possible bug where we where increasing the page number,\
in the beginning of the function. This could cause issues where the server returned an error or the request failed for any reason.
Now we are increasing the page after the success of the request.

We have also added a comment regarding the setHasMore logic to provide more clarity around it.
cursor[bot]

This comment was marked as outdated.

AlexNti added 2 commits July 11, 2025 10:33
We are making a change where in some components we where navigating to cats-gallery instead  the root of the application "/".
This is not breaking change, as the "/" is equal to "cats-gallery" as we have modified nextjs to treat as root the "cats-gallery".
The reason of this change is to make it more obvious to the reviewer that the navigation leads to cats-gallery.
cursor[bot]

This comment was marked as outdated.

Copy link
Copy Markdown

@gpositive gpositive left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @AlexNti - Thanks a lot for your effort mate! :)

I've checked your assignment and I have some questions for you:

  • Why did you choose Next.js over plain React (e.g., Vite + React Router etc)? Could you walk me through the trade-offs for this project’s needs?
  • I noticed that for some requests (ie getCatsImagesList) you're using a time based invalidation (ie: 600s) but for some others (ie: favourites) you're using a tag based invalidation. What was the reason behind this decision? How would you scale invalidation in general?
  • What are the pros/cons of calling the NEXT_PUBLIC_API_KEY from the client vs proxying it through server actions/routes?
  • I've tried running the tests but this failed. Do you have any guidelines on how can I run them?
  • If you had some more time, what would you have done differently?

Again thanks for your submission and looking forward to your answers!

Comment thread src/components/tabs.tsx
type TabItem = {
label: string;
href: string;
icon?: string;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this prop used anywhere?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch, its not used my intention was to add icon to the tabs but I forgot 🙃.

Comment thread src/hooks/useFetch.ts
fetchFn: (args: Args) => Promise<ApiSuccessOrError<T>>,
options: UseFetchOptions = {}
) {
const { fetchOnMount } = options;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that this option is not used anywhere in the source code. Can you verify if this is correct?
Could you help me with the reasoning behind this option? What does this do and what are the pros/cons?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed this is no longer used, Initially i was using it, but after making a small rethink e.x in the favourites component in the code I decided to follow a different strategy and all the initial fetches to happen on the server, so the fetchOnMount prop became lets say deprecated by that decision.
The fetchOnMount prop gives developers the flexibility to choose whether or not a query should be executed on mount. The standard behavior of this hook is to not execute the query automatically users need to trigger it manually by calling the execute function. This is the approach used in components that implement pagination.

@AlexNti
Copy link
Copy Markdown
Author

AlexNti commented Aug 12, 2025

Hey @AlexNti - Thanks a lot for your effort mate! :)

I've checked your assignment and I have some questions for you:

  • Why did you choose Next.js over plain React (e.g., Vite + React Router etc)? Could you walk me through the trade-offs for this project’s needs?
  • I noticed that for some requests (ie getCatsImagesList) you're using a time based invalidation (ie: 600s) but for some others (ie: favourites) you're using a tag based invalidation. What was the reason behind this decision? How would you scale invalidation in general?
  • What are the pros/cons of calling the NEXT_PUBLIC_API_KEY from the client vs proxying it through server actions/routes?
  • I've tried running the tests but this failed. Do you have any guidelines on how can I run them?
  • If you had some more time, what would you have done differently?

Again thanks for your submission and looking forward to your answers!


Hey @gpositive , thank for your review! In the following section I am addressing your questions.

Why did you choose Next.js over plain React (e.g., Vite + React Router etc)? Could you walk me through the trade-offs for this project’s needs?

I chose Next.js over Vite + React router because it includes React features built in, like RSC and server actions. My goal was to move as much logic as possible to the server and keep components simple for example, the toggleCatFavourite function I use to toggle a cat’s favourite status. React Router also supports RSC, but only in experimental mode.

Next.js also has a file-based routing system, which I find easy to use. Another benefit is its caching pages that don’t change often, like the breeds page, can be cached and served faster. As the cats gallery app is an app that uses alot of images we can benefit from the image optimazation that nextjs offers.

For the trade-offs, if I had used Vite + React Router, I would need to move the server logic to the frontend. I would also need to set up my own caching (e.g., tanstack query) and handle image optimization myself if needed.

I noticed that for some requests (ie getCatsImagesList) you're using a time based invalidation (ie: 600s) but for some others (ie: favourites) you're using a tag based invalidation. What was the reason behind this decision? How would you scale invalidation in general?

Correct those two are different revalidation strategies. Using tags allows us to create a more complex cache key and easily revalidate all content associated with that key, simply by knowing the key itself. In our case, we generate the tag key based on the user ID, and when the user adds something to favourites we invalidate the cache.
Time based revalidation is used in situations where the cached data doesn’t change frequently or does not changes based on user actions. In those cases, we let it invalidate automatically after a certain period of time.

When it comes to scaling, we should treat each situation as a standalone case. Sometimes we need to trigger cache invalidation on demand, while other times it’s fine to let it expire automatically after a set period.

What are the pros/cons of calling the NEXT_PUBLIC_API_KEY from the client vs proxying it through server actions/routes?

By moving it to the server we are hiding the API key from the users, so this offer more protection. To be honest i shouldn have omited the NEXT_PUBLIC prefix as by adding it it allow us to use the api key on the clinet as well and accidently expose the api key.

I've tried running the tests but this failed. Do you have any guidelines on how can I run them?

As the tests are e2e, and depend on a running server you need have an application that runs on localhost:3000. Please check the read me that I have included in this PR under the section How to run tests.

If you had some more time, what would you have done differently?

I would change the way the modal works in the cats-gallery by adding a carousel/swipe, now you click an image and a modal pops up, if you want to see the next cat you need to close the modal and click the next image from the grid, with the swipe/carousel view a user would be able to move easily to next/previous cat images without having to close the modal.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants