Responsive github API user search api with dark/light theme switch

Community feedback

• 

  Candela Bramucci•680

  @bramuccciPosted about 3 years ago

  Hi Leon, this solution looks amazing! I see it in the newsletter and I think it's great. In fact, I'm studying it so thanks for the readme!! The dark and light theme looks amazing too, just a question: The theme preference is not saved in the localStorage? When I reload the page, is always dark. Please keep coding! This is awesome.

  Marked as helpful

  1

  ---

  

  Leon Michalak•545

  @NinjaInShadePosted about 3 years ago

  @Liltanie Hey! Which newsletter, as I'm not aware of this :D

  Regarding theme storage, it is saved In localstorage. I have had issues on IOS though where you had a dark OS preference and it overrides localstorage. I have some ideas on what this could be and will work on a fix.

  What device/OS were you using?

  EDIT: It's fixed now :)

  Thanks for the comment! :)

  1

  

  Candela Bramucci•680

  @bramuccciPosted about 3 years ago

  @NinjaInShade Oh yes, it's fixed! Haha. I'm using windows. If you click the logo of Frontend Mentor in the web and scroll down, there will be a section with the heading "SUBSCRIBE TO OUR NEWSLETTER". I recommend joining. And you're welcome 🥰

  1

  

  Leon Michalak•545

  @NinjaInShadePosted about 3 years ago

  @Liltanie Yes, it was some logic mistake in my code actually, not OS specific like I first thought haha. Thanks, I've subscribed :)

  1
• 

  T

  Matt Studdert•13,611

  @mattstuddertPosted about 3 years ago

  Nice work on this challenge, Leon. You've done an awesome job!! 👏

  Your solution looks great and responds really well to different screen sizes. I'd recommend reviewing the accessibility and HTML validation reports to try to resolve those issues.

  Also, here are a couple more pointers:

  • In my opinion, you're overusing heading elements in this app. I'd say only the name (h1) and the repos, following, and followers (all h2s) are headings in this design. Heading are supposed to provide an outline for the content and act as titles for specific regions. In this design, the name is the main heading for the card, and the others are headings for the stats. None of the other content makes sense as a heading.
  • This is purely semantics, but I'd say a ul would be best for the information at the bottom of the card. It would also help screen reader users by announcing "List item 1 of 4", etc.

  I hope that helps. You've done a brilliant job. Keep up the great work! 👍

  Marked as helpful

  1

  ---

  

  Leon Michalak•545

  @NinjaInShadePosted about 3 years ago

  @mattstuddert Thanks, Matt! I'll get to doing that. I followed the design file to which elements were headings, so I guess that's not the way to do it :)

  Appreciate the feedback!

  0

  

  T

  Matt Studdert•13,611

  @mattstuddertPosted about 3 years ago

  @NinjaInShade, ah OK, that makes sense. Yeah, the design file is always just a visual guide. The HTML semantic and structural decisions are open for the developer based on best practices, including accessibility, etc.

  Glad I could help! 🙂

  Marked as helpful

  1