Develop#1753
Conversation
mateacademy-ai-mentor
left a comment
There was a problem hiding this comment.
Review Summary
Your implementation correctly uses event delegation on the #thumbs container and properly updates the main image source and alt text. The core functionality works as expected. The review makes a fair point about checking element types instead of class names. While your current approach works for the expected HTML structure (checking gallery__thumb and list-item__link classes), using target.tagName === 'IMG' or target.tagName === 'A' would be more robust and directly matches what the task describes: event.target will be "respectively, either img or a." This is a minor improvement rather than a functional issue—the code works correctly for the specified HTML, and this suggestion is offered as a "best practice" note for future reference. No changes are required at this time.
✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨
Footnotes
|
|
||
| let link; | ||
|
|
||
| if (target.classList.contains('gallery__thumb')) { |
There was a problem hiding this comment.
The task specifies that event.target will be an img or a element. Instead of checking class names, consider checking the element's tag name: target.tagName === 'IMG' for images or target.tagName === 'A' for links. This matches the actual task requirements.
|
|
||
| if (target.classList.contains('gallery__thumb')) { | ||
| link = target.closest('a'); | ||
| } else if (target.classList.contains('list-item__link')) { |
There was a problem hiding this comment.
Same issue as line 11 - list-item__link is a specific class name that may not match the actual HTML. Consider checking target.tagName === 'A' instead.
No description provided.