Sophie
@ippotheboxerAll comments
- @nelghzaoui@ippotheboxer
Hi there, nice solution! Here are some suggestions for what you could improve:
- I think a loading text/animation would be helpful for the user, when nothing happens it's a bit unclear whether it worked or not.
- The America region doesn't work: this is because the REST Countries API accepts Americas as a region, not America.
- If you select a filter and then click on the country, the filter gets reset. I think it would be a bit better if the filter region remained.
Hope this helps!
Marked as helpful - @dareyOG@ippotheboxer
Hi there, nice solution! It looks great with good animations.
Certain countries are giving an error: for example, Antarctica: Antarctica has many fields missing in the Rest Countries API, such as: no official currency, language, native name or capital. Make sure to check that if the response for fields is undefined, then just set a message such as "No official currency/language".
Kosovo also doesn't work: it doesn't have a tld (top-level domain) so you will also need to handle this case.
Also when you search for countries I feel like it would be good if you had an error message, for example "No countries found" if they searched something that has no results, I think it would be a bit more clear experience for the user.
Hope this helps!
Marked as helpful - @rahmi1016@ippotheboxer
Hey there, nice solution! It looks great and works well. One thing is that I can write a future date and it will still work - the year can't be in the future, but the day and month can be. Make sure to check the month and day against the current date, and if either is later, to raise an error since it's an invalid date. Hope this helps!
Marked as helpful - @LiciacodesWhat are you most proud of, and what would you do differently next time?
Most Proud Of: Implementing dark mode toggle with persistence using Local Storage. Handling dynamic routing to display country details. Ensuring a responsive and accessible UI with Tailwind CSS.
What I’d Do Differently: Optimize API requests for better performance. Improve error handling for API failures. Add search debounce to enhance user experience.
What challenges did you encounter, and how did you overcome them?Dark Mode Reset on Refresh
Challenge: The dark mode state would reset after a page refresh. Solution: Used Local Storage to persist the user's theme preference. Dynamic Routing for Country Details
Challenge: Ensuring correct data fetching and handling page reloads. Solution: Used React Router’s useParams to fetch country details dynamically. Styling & Responsiveness
Challenge: Making the UI look clean and accessible on all screen sizes. Solution
What specific areas of your project would you like help with?Right now, my project is complete, but I would appreciate feedback on:
Accessibility & UX – Is the dark mode toggle intuitive? Are there any improvements for a better user experience? Performance Optimization – Are there ways to improve API handling or reduce unnecessary re-renders? Best Practices – Any suggestions on improving my React code structure, state management, or component organization?
@ippotheboxerSome bugs I noticed:
- Light and dark mode don't work. The whole app is essentially staying in dark mode, only the region filter and search get changed when I toggle the mode. You're using local storage to store darkmode, even when I change it to false, it doesn't change to light mode. I think this could be because local storage sets everything as text, so maybe true is being read as "true" rather than a boolean. For example, you do:
const [darkMode, setDarkMode] = useState<boolean>(() => { return localStorage.getItem('darkmode' ) === "true" });
But you should check
localStorage.getItem('darkmode' ) === true
, since it's a boolean. Also instead of returning it like this, just setuseState<boolean>(false)
, and you could use useEffect to detect if the darkmode in local storage changes. You're basically always returning dark mode as true instead of reading it.- On dark mode, I can hardly read the text. On dark mode the text is supposed to be white, using a black and dark grey text makes it pretty hard to read, and when you search, I can't see the text that I'm writing.
- Between the large and small screen sizes, there's a sudden jump to 1 per grid then to 3. So when it first changes to 3, the height is way too high for the flags and all the flags don't contain the whole flag, just a small zoomed-in portion of the flag (this can be fixed with object-fit). I think it would look better if it switched to 2, and then 3.
- I feel like using local storage to set the theme could be a bit overcomplicated: I think it would be better to use a context to store the theme, and maybe if it changes you could then write to local storage.
- @abdalla-shakerWhat are you most proud of, and what would you do differently next time?
making a custom hook and implementing some new things in my code such as modal component and forwardRef and many other things I don't remember TBH.
What challenges did you encounter, and how did you overcome them?Opening the modal using context api was a pain but i cancelled that idea as the code didn't work asynchronously so i went back to prop drilling (well it's not prob drilling because i passed the functions to one component only) and i was gonna use useReducer instead of useState but the state updating functions was not complicated so I canceled that idea.
What specific areas of your project would you like help with?Any feedback is appreciated
@ippotheboxerHey there, nice solution!
First of all, you mentioned how using a modal was complicated: a modal is brought up when pressing on a country, but I think it would indeed be less complicated if you used react-router and instead took the user to a new page (e.g. /:countryName) when they press on a country, and then fetch the country name using useParams and then hitting up the API.
Also, searching for a country doesn't work when a filter is applied. This is because you have done it like this:
if (filterRegion) { FILTERED_COUNTRIES = countries.filter( (country) => country.region === filterRegion ); } else if (searchRegion) { FILTERED_COUNTRIES = countries.filter((country) => country.name.common .toLowerCase() .includes(searchRegion.toLowerCase().trim()) );}
Filtered region takes precedence over the search, so search will only work if there is no filter. Here is how you can do it so that both work:
let FILTERED_COUNTRIES = countries; if (filterRegion) { FILTERED_COUNTRIES = FILTERED_COUNTRIES.filter( (country) => country.region === filterRegion ); } if (searchRegion) { FILTERED_COUNTRIES = FILTERED_COUNTRIES.filter((country) => country.name.common.toLowerCase().includes(searchRegion.toLowerCase().trim()) ); }
I see that you have your filter and search context separated, but I feel like it's fine to have them both together in context, since they are both directly manipulating the countries that are being shown
Marked as helpful - @niklaus699@ippotheboxer
Hey there, nice solution! The UI looks clean and is responsive.
When I click on the confirm order, it's meant to clear the current order so that the user can restart their order and make a new one: however the cart data doesn't change at all and still displays the same products. Make sure that the data gets reset when the user presses "close" after the order was confirmed.
Also when you press on the small X by a food item in the cart, if you go and hover on the food item you removed, it still shows the quantity that was added. And if you keep pressing the decrease button to get the product quantity to 0, it still shows up in the cart with 0, but it's meant to disappear from the cart. Pressing X when this happens doesn't work either.
- @TrEv0rRrRr@ippotheboxer
Hi there, nice solution!
When you click on a card and get more info, the flag looks very low quality: in your code, you're using PNG to display this image. The rest countries API also offers an SVG, and SVG actually is higher quality: this is because SVG files are vector-based, so they can expand to any size without losing their resolution. It's better to use an SVG file on this page, and on the page where you search for countries, to use a PNG, because each card needs to have the same width and height for the card image, and the png images scale to any width and height better.
Marked as helpful - @Nitish-Thakur-05@ippotheboxer
Hey, nice solution! When you search for a country and there's only one or a few results, it ends up being a very massive card that takes up the whole screen. I think you should use CSS grid so that one card always takes up a certain amount, rather than expanding to the screen. Then with media query, you can change the amount of grid columns and their widths/heights at different screen sizes.
- @Effy1996@ippotheboxer
When I press on the shorten link button, shortening the link doesn't seem to show anything - in your code, you have <UrlList /> commented out.
You have your code to shorten the url in the frontend, but trying to hit up cleanURI won't work in the frontend due to CORS policy. You can set up a very simple backend with node and express, host it separately, and then use fetch with that backend route to get the response. If you're not sure how to use node or express, you can use chatgpt to help with it since this code will be quite simple.
Here's an example:
You will need to install Node.js.
In a backend folder, create server.js
const express = require("express"); const path = require("path"); const app = express(); const PORT = 5000; app.use(express.static(path.join(__dirname, 'build'))); app.use(express.json()); app.use(cors()); // Serve the static files from the React build folder app.use(express.static(path.join(__dirname, "build"))); // Endpoint to shorten URLs app.post("/shorten", async (req, res) => { try { const { url } = req.body; if (!url) { res.status(400).json({ error: "URL is required" }); return; } const response = await axios.post("https://cleanuri.com/api/v1/shorten", new URLSearchParams({ url })); res.json(response.data); } catch (error) { console.error("Error:", error); res.status(500).json({ error: "Failed to shorten URL" }); } }); // Catch-all route to serve the React app for all other routes app.get("*", (req, res) => { res.sendFile(path.resolve(__dirname, "build", "index.html")); }); // Start the Express server app.listen(PORT, () => { console.log(`Server running on port ${PORT}`); });
You can try using this locally, then you could deploy it on render.
Marked as helpful - @zhansayatazhibayeva@ippotheboxer
Hi, there seems to be a bug in your url shortening code.
When I give a link, it says "undefined" on where the shortened link is supposed to show. The copy button also doesn't work. In the console, it says "Failed to load resource: the server responded with a status of 429 ()". Error 429 means too many requests. The Bitly API that you are using, with the free plan, only allows 5 requests to be made per month.
It would be better to use the cleanURI, the api provided, since it doesn't require an API key or have any limits. However if you tried to do this on the frontend and found this didn't work, this is due to CORS policy: if you create a very simple backend that hits up this API and then have the frontend fetch it, you could get around this error. You could even just ask chatGPT to create it with node and express if you aren't very familiar with how to do this.
Also, I can still submit a link even if I didn't put a link in. This will inevitably lead to errors no matter what API you use, so I recommend using .trim() method around the input to validate it and check if anything was given, and then you could use regex to check if the link is in a valid format.
Hope this helps!
Marked as helpful - @AbdallhElzorkany@ippotheboxer
Hey there, nice solution!
There seems to be a bug with the cart: when I add a product to the cart, and then press on the X to delete that item, on the product itself it still shows the number of items: for example I add 3 tiramisu to the cart, remove it, but on the tiramisu it still stays 3 items are added.
The same thing happens if you press confirm order and then start new order: on the products, it still displays the amount that was selected. This doesn't get reset. Make sure that the state of these items is linked to the products that are shown.
Marked as helpful - @AmitFrontEnd@ippotheboxer
Hey there, nice solution! Your app works well and the styling is great.
One thing that you could improve is that when you start searching for a country, the filter for regions immediately gets removed: however I think it would work better if you could search for countries within a specific region. I believe this is because renderCountries can only either take a region or search at a time, but not both.
Also with the back button once you're on a country, I think it would work better to just take the user back to the homepage rather than the last page visited, since if you click through a lot of countries it will then take a while to get back to the homepage.
Also, a small thing, but I think it could also be useful to have a filter on the region that just says "all" or something like that, to go back to having no filter.
Hope this helps :)
Marked as helpful - @DocForLoopWhat specific areas of your project would you like help with?
Feedback is welcome!
@ippotheboxerGreat solution, it's very responsive and the design looks great at all screen sizes! Well done :)
- @RealKendprWhat specific areas of your project would you like help with?
I'd appreciate the help on how I can position the big squiggly line on the bottom.
@ippotheboxerHey there, nice solution! To position the big squiggly line, you can use absolute positioning on it and set
bottom: 0; left: 0;
You error handling works nicely, but I can not submit an image and the ticket will still be generated. Make sure to check if this is provided! Also when provided, it doesn't show on the ticket after submission.
Marked as helpful - @jakubsmid22@ippotheboxer
Hello, your solution looks great and very accurate to the design, and works well! One thing is at day 29, 30, 31 work with February which obviously isn't possible. An easy way to catch this error is to do the following:
if (!day) { newErrors.day = 'This field is required'; } else if (day < 1 || day > 31) { newErrors.day = 'Must be a valid day'; } else { // Ensure the day is valid for the month and year const maxDaysInMonth = new Date(year, month, 0).getDate(); if (day > maxDaysInMonth) { newErrors.day = `Must be a valid date`; } }
Hope this helps!
Marked as helpful - @vidyaa01What are you most proud of, and what would you do differently next time?
I'm proud of how structured and efficiently I could think and code. Definitely practice makes perfect.
What challenges did you encounter, and how did you overcome them?Following are the challenges I faced:
- Image alignment and fitting it in the recipe card
- Making the page responsive
How did I encounter?
- Made the image margins auto and gave it a max-width
- Studied media query
- When to use % and when to use px, vh etc?
- Is there any standard way where i can learn to write neat code?
@ippotheboxerHello, nice work with this project! The components on the recipe scale well responsively. Here are some tips!
At full screen size, the recipe is not in the center of the page. I would recommend using flexbox to center and position divs. For example, on main in styles.css, you could add the following css properties:
display: flex; justify-content: center;
This will align it in the horizontal center of the page, which means you don't have to set margins and width to position it in the center. Regarding use of %, px and vh:
VH: viewport height is usually used on the main container (e.g. body or main) and it is usually set as
min-height: 100vh;
to ensure it takes up the whole screen.px: Using pixels is usually best for setting font sizes or image width/height. You can use media query to change pixel sizes at different viewport sizes.
Percentage: this is usually best for ensuring a div or element on screen only takes up a certain amount, for example with your component you could make it so that the img takes up 100% width, and then apply margin around.
You also shouldn't set min or max widths to the body, but rather the custom div components such as the recipe component.
- @IdiruWhat are you most proud of, and what would you do differently next time?
I could improve it with some animations and I wanted to do a 3d version of the ticket but I had to move forward.
What challenges did you encounter, and how did you overcome them?No big challenges ti be honest.
@ippotheboxerWell done, this solution looks great! I love the animation and that you keep error handling as states, and don't allow form submission before all inputs are in valid formats. Nice work! 🥳
For the errorhandler, I would recommend using regex to check if your inputs are in valid format. Regex are regular expressions that match patterns in text, and they are a lot more complex and also check structure rather than just using .includes().
Here are examples you can use for email and github username.
const emailRegex = /^[^\s@]+@[^\s@]+\.[^\s@]+$/; const githubRegex = /\B@([a-z0-9](?:-(?=[a-z0-9])|[a-z0-9]){0,38}(?<=[a-z0-9]))/gi;
To implement it with your code, you can do it like this.
const errorhandler = () => { if (!form.email.trim()) { setError(true); // no input given } else if (!emailRegex.test(form.email)) { setError(true); // The test() method tests for a match in a string. If it finds a match, it returns true, otherwise it returns false } // repeat the same for github username regex, and for fullname also check that it's not empty }
I think this would also save you some code, since you also have the checkFormCompletion function checking to see if there's an input given, but just checking for errors and including an empty input as an error would let you just use one function for error handling rather than having two.
Marked as helpful - @backendfrontflipWhat are you most proud of, and what would you do differently next time?
Being able to fetch my api url using const restUrl = "url-address". What i'd do differently is to not just fetch the url while await async but also include a try and catch function to be exact
What challenges did you encounter, and how did you overcome them?Trying to figure out the all countries card section. it wasn't displaying all of it until i flex items-columns and also the media responsiveness for different screen sizes. had to us the md, lg and sm style property to debug that with my tailwind css
What specific areas of your project would you like help with?i personally think i can help with all areas of this projects as it took me 3 days to successfully program it.
@ippotheboxerHi, what you've done so far is really great! The search works and the filter by region. However you didn't incorporate one of the keys parts of the app, which was clicking on a country article to get more specific information displayed. Here is some advice on how to implement it: In your App.jsx, you aren't using BrowserRouter to get dynamic routing, so currently it would be impossible to implement this other page. In the App.jsx, you could make it look like this:
return ( <div className="min-h-screen"> <BrowserRouter> <Routes> <Route path='/' element={<Home />} /> <Route path='/home' element={<Home /> } /> <Route path='/:countryName' element={<SpecificCountry />} /> </Routes> </BrowserRouter> </div>
And then create a Home.jsx in a pages folder. Then simply move the code you currently have in App.jsx to Home.jsx. Now when you are mapping through in AllCountries.jsx, wrap it with
<Link to={`${country.name.common}`}> </Link>
. Now you can create a new page called SpecificCountry.jsx and include all the more specific information. Now on that page useconst { countryName } = useParams();
to get the countryName from the link. You can make a get request to the api with that name now, and get all the specific information. Then you can include a back button on it withconst navigate = useNavigate(); ... <button onClick={() => navigate("/")}>Back</button>
To go back to the homepage. Hope this helps!