Skip to content
OverviewSolutions
0
Comments
2
Federico
@efedericomedina

All comments

  • Federico•20
    @efedericomedinaPosted over 1 year ago

    Really nice work Mateusz,

    the axios.get is repeated multiple times, the only difference is that the api_key changes, how about creating a function that receives as input the api_key, then you avoid repeating the same code multiple times. maybe you can use a constant for the url as well.

    0
  • Federico•20
    @efedericomedinaPosted over 1 year ago

    Hi, really nice work Anisha, the ui looks good and its nice to see how you used the correct html tags and the if statement within the functions.

    I leave you some points to consider: In the html file, the img tags don't have the alt attribute set, maybe adding a text to those would be nice and cover a part of accessibility.

    In the css file, what do you think about using variables to store the colors? and then use those variables whenever you need to indicate a color instead, if the color happen to change in the future then you only need to update the variable value in one place instead of having to update the color everywhere. check(https://www.w3schools.com/css/css3_variables.asp)

    In the js file the function focusState, maybe could be simplified, I see that the first function's argument is the one that changes color only, maybe instead of passing all the params, only passing that id that changes colors on focused event and the rest of the ids, you could use a for-loop to change the colors since they are all the same.

    afterHoverSubmit and hoverState are the same functions though the names changes, maybe having one function with a more generic name will allow you to reuse your function.

    Maybe instead of assigning the hardcoded colors on the functions, you could use constants to declare the colors outside the functions and the use them whenever needed.

    Marked as helpful
    1