React, Ipify, Leaflet.js

Community feedback

• 

  Matthias•235

  @MatthiasSmithPosted over 3 years ago

  Hey @OlehTovkaniuk, great job on this! Your solution works well, and your code is simple to follow and read too!

  Couple of things I noticed:

  • There seems to be too much padding inside of the "banner" element when viewing the solution on larger screen sizes.
  • Setting the "outline" property to "none" on the text and submit input elements makes it difficult to tell which element has focus when I use tab keys to navigate the page. Perhaps you could add them back for when people navigate using the keyboard using :focus-visible? Here's the MDN reference for it: https://developer.mozilla.org/en-US/docs/Web/CSS/:focus-visible

  I'd also like to offer you the same challenge you posed to me, which is to make your API token invisible to the client. 😉

  Let me know what you think.

  Best,

  Matthias

  1

  ---

  

  Oleh Tovkaniuk•505

  @OlehTovkaniukPosted over 3 years ago

  @MatthiasSmith Thanks for well-aimed suggestions! I did the changes 😊 About API token, you caught me 😁 I haven't that done because it requires understanding of serverless logic but instead of learning it I'm going to dive deeper in React and related technologies.

  1

  

  Matthias•235

  @MatthiasSmithPosted over 3 years ago

  @OlehTovkaniuk Changes look good! 🙂

  Yes, I understand about diving deeper into React. When you do give it a chance, you might be surprised how simple it is to write a few functions (using node.js or another technology) to handle api requests and the like. Not a bad skill to have under your belt either. 😉

  Happy coding! 😎

  1

  

  Oleh Tovkaniuk•505

  @OlehTovkaniukPosted over 3 years ago

  @MatthiasSmith Thanks, happy coding 😊

  0