Skip to content
OverviewSolutions
4
Comments
7
Damjan
@iDamjan

All comments

  • Héctor Figuereo•370
    @hector535Submitted about 1 year ago

    Built with 🔨

    • Vite
    • React.js
    • TypeScript
    • React-Router
    • SASS Modules
    • React-Virtuoso

    I'm always open to suggestions on how to improve on any aspect you think I'm lacking. If you also find any bugs, please don't hesitate to let me know; I would appreciate it a lot.

    I hope you guys enjoy it. 😊

    DevJobs App | Vite | React | SASS | TypeScript | React-Virtuoso

    1
    Damjan•220
    @iDamjanPosted about 1 year ago

    Hi,

    okay wow, awesome, i am really curious of your coding experience, and how long have you been doing it. You also added test, which is really awesome.

    I would add really minor things, my opinions:

    • Don't rely on Viewport in the JSX, i think that should be always handled by CSS/SCSS and media queries (in case there is some specific reason i am missing)
    • I think you should put all the views/pages in a Views folder or Pages folder and any other view as well, i can see that you added this in Router folder but that may be really confusing :D . As this is a becoming a medium project, with more than 10 components its really hard to navigate.
    • I think that you really tried to separate things and make it simpler and you ended up with too many components, and its really hard to navigate and review this. Ideally this should be constructed with up to 5 components i think.
    • If there is nothing to load more, then disable the button or hide it.
    • Its weird that you can only scroll inside of the jobs, it comes intuitive to scroll in the outside as well
    • Interesting update would be to make the search to be live and happening immediatly, its nice improvement.

    I hope my comments will help and not only make you angry (I know how comments on PR can make a person angry), but remember this are just my thoughts, your code is awesome and really good job :))

    Regards, Damjan

    Marked as helpful
    1
  • Damjan•220
    @iDamjanPosted about 1 year ago

    Hi,

    Good job finishing this challenge!

    In case you are keen to feedback i will provide some comments.

    • You should never state checking the window size, only to change an image. You should do this in CSS using media queries, it would be a lot simpler and easier for you.
    • Try to make the <App> component as clean as possible and to only contain other components and not a custom logic, here you can extract all the logic and JSX for the Input to be a separate component.
    • Form/input validations its really common thing, although you achieved to validate the input i would suggest couple of things. You should validate on every keystroke and update the value (like you are currently doing) and show the validation classes onBlur, or when the user clicks somewhere else outside of the input, AFTER he started typing. This is really good and common approach. (fun thing to investigate is the validity object, which is native object from HTML5 that can help you with validation really easy :)) )
    • I know this is just a practice challenge but just try to clean out all the comments which you have left, its a good practice to have :D
    • To make your life easier with naming stuff, especially CSS classes as that is the hardest thing in programing, try looking into the BEM standard of naming, it solves this problem for you

    I hope this comments helps you and not just make you nervous :p Regards, Damjan.

    Marked as helpful
    1
  • Damjan•220
    @iDamjanPosted about 1 year ago

    Hi Oryan,

    You will find out in a short time that actually hardest coding challenges are related not to actually coding but coming up with names :D Starting from naming function to CSS classes.

    The thing that helped me the most was following a standard or lets say architecture, and thats the BEM standard, which stands for Block-element-modifier. With this standard you will not have to think much as the standard it self will think for you.

    You can read the docs here : https://getbem.com/introduction/ BUT basicly how it goes its you have a block in the HTML lets say <form> and some children inside <button> or <input>, the main class of the block will dictate the main name and the children or elements will be the name after the block ex. Form---button, Form--input or Form--contactButton and then the M in the BEM standard which stands for modifier will be for example if you have some modified state ex. Form--contactButton-disabled, Form--input-error.

    There you go, CSS naming solved :P I hope this helped.

    I will also suggest for you to try SCSS, which is just another way to write CSS. It changed my life and i would never come back to write normal CSS. :))

    Marked as helpful
    1
  • Damjan•220
    @iDamjanPosted over 1 year ago

    I think your project looks great, nice structured css. I would add a suggestion. Look into BEM naming convetion of classes, take a look at SCSS and use that instead of regular CSS (you will love it), pay attention on acesibility meaning, if its intended to be a button, add a button html element not link (regarding to the navbar cart), that should be a button and not a link, since it doesn't navigate anywhere.

    Overall nice, good job! I hope my makes sense and will help you :)

    0
  • Damjan•220
    @iDamjanPosted over 1 year ago

    Overall it looks fine to me. I would suggest though minor things, which are mostly preference.

    Don't leave console log in a code and actually do something with the error, maybe set some boolean state and display error message on the screen. One thing i would do also to add a general loader everywhere, which will be handled by the loadData() function in a finnaly block after the catch so: catch(e) {setError(true)} finally {setLoading(false}.

    I would also add better naming, because i think most of the things are not very clear. and maybe styled components in a separate folder outside everything, which i think its a prefered way of using styled components.

    Let me know if this makes sense. Overall i really like the project, my version when i was doing this was a total trash :)))

    Marked as helpful
    0
  • Damjan•220
    @iDamjanPosted almost 2 years ago

    Hey Kumar,

    you can do this with Media queries. Just add @media (max-width:40em) { background-position: (desired position)} this will trigger different styles starting from 40em, or you can set your desired width trigger.

    I hope this was the answer you were looking for!

    Let me know if i can help with anything else.

    Happy holidays!!

    0
  • yanyan_alien•170
    @yanyan-alienSubmitted almost 2 years ago

    Any feedback is welcome!

    Also, after completing this assignment, I've noticed that a lot of the completed ones are done with React or some kind of framework while I completed without one. Why is this so?

    Getting country data with Fetch API

    2
    Damjan•220
    @iDamjanPosted almost 2 years ago

    Hey!

    Impressed that you did this with only vanilla javascript... I encourage you to try some frameworks, like React or maybe Vue and you will find that building websites like this will get much easier. Also worth mentioning is that most of the companies are working in some framework so hiring would get much easier if you master one of the javascript frameworks.

    Overall good job, understanding the core essentials and pure vanilla javascript is crucial, with that knowledge you can learn and tackle any javascript framework easily.

    I hope you find this helpful!

    Happy holidays!!

    0