Art Gallery - Responsive Design with CSS Grid and Leaflet JS

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hello there! 👋

  Good job on this challenge! 👏

  Some suggestions from me.

  • I would not recommend opening a new tab for the site navigation or internal navigation. It can be annoying if the site has multiple pages or if the users want to navigate back. It means that the users would be ended up with a lot of tabs. My recommendation is to open a new tab for navigating to another website.
  • I recommend adding rel="noopener" to any anchor tags that have target="_blank". This is a security essential for external links. I suggest reading the web.dev article to learn more about this.
  • One of the most important contents on the page is the logo. It's not decorative, and not the one with blank alt="" text. So, I recommend adding an alternative text.

  One last thing, I notice a horizontal scrollbar at 1280px width. It's because of the margin on the .gallery. So, I recommend trying to fix this issue.

  That's it! I hope this helps. Happy coding! 😁

  Marked as helpful

  0

  ---

  

  P

  a-woodworth•290

  @a-woodworthPosted over 2 years ago

  @vanzasetia Yes, that media break for desktop was tricky as the image switch happens before the desktop break. I'll circle back--probably need to adjust that section to use calc and subtract the padding.

  As for the rel="noopener", the article you linked to is not true anymore, unless you need to still code for older browsers like Internet Explorer. See MDN documentation here: https://developer.mozilla.org/en-US/docs/Web/HTML/Link_types/noopener

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @a-woodworth It is important to make sure that the users that still use older browsers like the Chromium below version 88, which doesn't automatically get noopener behavior by default get the protection.

  If you read the article that I shared, it says clearly that the purpose of putting rel="noopener" is to protect users that still use legacy browsers.

  As of Chromium version 88, anchors with target="_blank" automatically get noopener behavior by default. Explicit specification of rel="noopener" helps protect users of legacy browsers including Edge Legacy and Internet Explorer.

  Not all users are updating their browsers for a lot of reasons (such as the users still use old Windows, etc).

  So, it's best to add the rel="noopener". 😉

  0

  

  P

  a-woodworth•290

  @a-woodworthPosted over 2 years ago

  @vanzasetia Those Internet Explorer users will not be able to view this site anyway. It is built using Grid, including use of grid-column and grid-row for the art layout.

  I appreciate your point of reaching a greater audience. Developers need to make trade-offs with features and should be as inclusive as possible, including using polyfills, fallback code, etc if they need to support older browsers.

  However, some of us do these Frontend challenges to show off new browser features. I selected this challenge to test my Grid capabilities. If the art gallery doesn't work for IE users, oh well ¯_(ツ)_/¯

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @a-woodworth The users of the legacy browsers don't have to be Internet Explorer users. They could be the users who use old Google Chrome or Firefox browsers for some reason.

  But, if you still insist on not adding rel="noopener". Then, it's okay. Just, be aware of the issue. 🙂

  0