Skip to content
OverviewSolutions
3
Comments
11
Janos Takacs
@JT1974

All comments

  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Yeimy 👋

    I think something went wrong during the deployment, because when I click a card, it does not open/display. I tried it in Chrome, Safari and Firefox too. 😞 Maybe a path issue?

    I also couldn't filter the regions, it just does not react, although the search works fine.

    Meantime I looked into your code and it looks awesome. Very well organized and structured, split, separated into smaller chunks, great, great job for the first sight! 🙂 👍

    I hope, I can review the fully working solution, because so far I like your solution the most...

    Good luck with that! Cheers

    Marked as helpful
    1
  • Can•235
    @escarcanSubmitted over 2 years ago

    My first project with React, i struggled with hooks at the beginning, how can i improve my code? any comments are welcome :)

    RestCountriesAPI with ReactJS

    1
    Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Carlos 👋

    Your solution looks perfect! Great job! 👍 I like the white space and the responsiveness too. Even if you struggled with hooks, the app works perfectly and your code is also clean and easy to read.

    In case you would like to improve it, you may (or may not) want to consider some of my thoughts:

    Tip #1: for such small projects, maybe downloading icons instead of using react-icons can be a good idea to reduce dependencies

    Tip #2: also, for only 2 routes, maybe using conditionals instead of the great (tiny) wouter can be another good idea, which may not reduce your bundle size too much, but probably it's easier and good enough for small projects

    Tip #3: a stateful header component with dark class added to the body could be an alternative for switching themes, although your solution looks and works perfectly too

    Overall, very nice solution, congratulations! 👏

    Marked as helpful
    1
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Queen-codes 👑👋

    Great work, congratulations! The website looks stunning! 👏 Nice SASS with module imports and easy-to-read semantic HTML! I like that a lot.

    If you would like to further improve the site, I would suggest a few things, that may help you.

    The main navigation links aren't clickable when hovering over the bottom of the area (because hyperlinks are not covering the list items fully). Tip: it works for the submenus, maybe the same logic could be implemented in the main nav too?

    The alignment of the different items is quite important in this design. For instance the header horizontal line (your <div class="border"...) is aligned with the page title and in some cases the content too. Tip: the simplest fix is using css-grid e.g., that aligns your items properly.

    White space is also lacking in some cases, and some images are a bit too large, which pushes the content out of the viewport in some of the required screen sizes. The typography is not always inline with the design either. Tip: Figma provides easy-to-use tools to check alignment and measure whitespace, and provides the css properties of the elements in the Inspect tab. 👍 Makes the coding a lot easier. To learn about that, look for Gary Simon's videos, but I guess you can find tutorials on freeCodeCamp.org too.

    The horizontal line is sliding off in desktop view. Tip: this could be fixed by adding a :before pseudo-class to the main navigation, instead of using a separate container just for the border.

    The submenu link jumps up a bit when I hover over it, due to the border-bottom. Tip: it can be fixed by setting a border by default, but making it transparent and during hover you just change the border-color. Nice little trick to make it look even better. 😉

    Overall, I like you site, I think it's beautifully created, so well done! 👏🙂

    0
  • Prantik•660
    @prantiknoorSubmitted over 2 years ago

    Hi there.👋 I have completed the space tourism website challenge with help of scrimba course & Kevin Powell.

    I have learned a lot of things. & It is my first multipage website. So it was tough for me.

    Feel free to give suggestions & feedback. I will be happy if you help to improve

    Responsive Space tourism website

    1
    Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Prantik 👋

    Nice solution, congratulations! 👏

    I have a few ideas how you could further improve your code to make your site look even better.

    On mobile, the EXPLORE button is quite big, actually it takes up approx. 40% of the height of the screen. Even though it's a CTA, it might be a little smaller to create some whitespace.

    The same goes for the planet's name, the typography there doesn't follow the design (it's 100px in mobile, 80px in tablet, then again 100px in desktop view). EUROPA doesn't even fit to the screen. You can set the font-size, along with white-space and other properties with media queries.

    I'm not sure why, but the layout changes at 560px, and right after that, at 560px the tablet view kicks in. At this width, there's no white space in case of the Technology heading and hardly any in case of the other pages. And to change to desktop view at 720px is definitely too early, because it doesn't fit to the screen.

    These are smaller and easy to fix issues, that could be fixed by adding a few more utility classes (I see that you like that).

    Overall, it's a nice website, that you can be proud of, so keep up the good work! 👍🙂

    Marked as helpful
    0
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Wojtek 👋 (I'm Janos from Budapest 🇭🇺)

    Yes, you're code is beautifully clean and readable, I really love it! I will fork it and learn a lot from it (I quickly browsed it, but I saw a ton of beautiful solutions). Congrats man! 👏 I have a nice little trick for you to further improve it. Instead of your displayCommasInNumber.ts utility, try this little snippet: new Intl.NumberFormat('en-EN').format(population). This will give you the same result. I hope I could add something to your cool solution! 😂

    About the UI, I feel a lack of whitespace in mobile view, probably because you stretch the view, instead of having fix card sizes. Also, box shading is missing. Not a big deal though. What I mainly miss is the header in detail view (I cannot switch modes), and the border countries are only labels and not clickable buttons. If you need some visual guidance, check out my solution. There's one thing I was advised of too, from any countries (no matter how far you clicked into neighbors) you should be able to go back to the home page by the click of a button (maybe HOME). Here it is: https://jt1974.github.io/rest-countries/

    To sum it up, I really like your code! Congratulations 👏, and if you'd like to work on something bigger in a team, I would be happy to join! 👍

    Marked as helpful
    1
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Tasos, Your solution looks awesome on desktop and even on 5K. Nice job! 👏👍 The semantic HTML looks and reads very well, congratulations! On mobile however, it falls apart, so you may want to look into that for smaller screen sizes. The issues start at around 800px which is quite large screen still (tablet, wide mobile size). Anyway, it's a great start just needs some styling for smaller screens. Congrats! 😀

    2
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey TRISHlove,

    You've done a great job, your project replicates the original pretty well. 👏👍 The HTML has a nice structure, you're using semantic HTML mostly, what makes it easily readable, so you may not need those HTML comments, because the code is structured well enough, so it doesn't provide any value. Not a big thing though. To make it even more semantic you could try using sections instead of divs for clearly separated contents. And finally there's a great video from Kevin Powell about HTML structures (I don't remember the title...), where he explains why it's not advisable to have more h1's in one doc, and how to best structure the code. It's an interesting video, you may like it. Anyway, it's a very nice solution so congratulations! 😀

    Marked as helpful
    1
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hey Karthik, I'm sorry I was probably not clear enough with those 2 buttons... 🙃 I didn't mean the RULES and the EASY/HARD buttons you have at the bottom of the page. Those are cool.

    What I meant was that after the player chose a hand (scissor, paper, etc.), you had the player and computer buttons slide in to the center of the screen. Your intentions were to hide both the player's and the computer's choice at the left and right of the screen, outside of the screen, however on large screens (like an iMac) the buttons are still inside the screen (because it's that wide), so I could see them before they slid in.

    Again, it's really not a big issue, I don't think you should worry about it, just wanted to clarify that. Congrats, it's a nice solution! 🤓

    1
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hi Aniket,

    You have done a good job, I would just add a few comments: You left the CRA readme file in the folder. You maybe missed this part in the challenge details... The points can go down below 0, which is not according to the rules, so you could maybe review your code and build in a guarding statement for that. The icons seem to be a little big, you could maybe reduce their sizes, because the button borders are currently cutting off their outer areas. The rules modal looks weird, and it's not readable. I'm not sure why that big circle is around it, and the title is also missing... Did you try to display the triangle svg, because I cannot see it (I couldn't find it in your code either)? The app is working well (exc. for that negative score issue) in all resolutions I checked, so congratulations! Good job!

    Marked as helpful
    0
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hi Giwa,

    You definitely love React components... 😂👍 That's great, I love them too. Your solution is working well, there's only a few suggestions (smaller ones) I would make: First of all, you left your console logs in the code, so it's logging your states for every click. The app is still functioning, so it's just something I you could consider. Secondly, the styling could be a bit more responsive, there's a lot of unused space in almost every resolution. You could try flexbox, and/or a few media queries, even in your styled components, it would definitely improve UX. Thirdly, a small issue, you forgot to remove the React icon and quite a few CRA assets from your folder structure. It was funny when I found a Netflix logo in one of your folders... 😂 🤷‍♂️ Anyway, overall I think it's a good and working solution, maybe the UI could be a little bit improved. Congratulations!

    0
  • Janos Takacs•230
    @JT1974Posted over 2 years ago

    Hi Karthik,

    I'm on iMac and it looks a bit strange, that I can see the 2 buttons before they slide in to the center. On smaller screens it looks ok, so I think it's not a big deal (anyway the target display was 1366px wide), but other than that it looks good! You even solved both the 3 and 5 button versions which were both well done. Congratulations!

    1