[Work In Progress] Image Gallery Component #234
Conversation
There was a problem hiding this comment.
In general, this is great work. I especially appreciate how you were able to do all the code walkthrough work without any assistance (it is not exactly the easiest library to work with).
Please expose this as an NPM Module (this will require us to ensure the library is in index.js).
js/modules/es4/image-gallery.js
Outdated
| slide.setAttribute('aria-hidden', i !== index); | ||
| if(i === index) { | ||
| const msg = slide.getAttribute('alt'); | ||
| slideAlert.innerHTML = `<p>Now showing image with ${msg} </p>` |
There was a problem hiding this comment.
Let's remove the <p> tags here. It may interfere with the screen reader alerts in some browsers.
| <img src="images/image-gallery/02-laser-disc.jpg" alt="A CD with colorful reflections on its surface, resting on a black case." class="slide"> | ||
| <img src="images/image-gallery/03-laser-disc.jpg" alt="A large disc with rainbow reflections on its surface, placed on a cluttered table." class="slide"> | ||
| <img src="images/image-gallery/04-laser-disc.jpg" alt="A person holding a large disc with rainbow reflections on its surface." class="slide"> | ||
| <img src="images/image-gallery/05-laser-disc.jpg" alt="Two discs with rainbow reflections, one large and one small, on a white background." class="slide"> |
There was a problem hiding this comment.
In general, this is a good demo. I want to make two variations of this component on the page:
- A gallery without visible captions under the photos (basically this demo ... but I want to use different images. I would like to add product shots of the Space Invaders miniature arcade game. The pictures are in this gallery, alt text and captions are in the description for each image.
- A gallery with visible captions under the photos (this one will use these images). I would like you to use the images in this gallery (alt text and captions are in the description for each image).
content/body/image-gallery.php
Outdated
| <img src="images/next.svg" alt="Display Next Image"/> | ||
| </button> | ||
| </div> | ||
| <div class="gallery-alert sr-only" role="status" aria-live="polite"> |
There was a problem hiding this comment.
Let's change the aria-live to "assertive".
content/body/image-gallery.php
Outdated
| <?php includeStats(["isNPM" => true]); ?> | ||
|
|
||
| <div id="example1" class="enable-example"> | ||
| <div class="gallery"> |
There was a problem hiding this comment.
Let's add role="group" and aria-label="Photo Gallery". If we do that, screen reader users will hear that they are inside a photo gallery when they tab into this component.
content/body/image-gallery.php
Outdated
| { | ||
| "label": "Add aria-hidden to slide image", | ||
| "highlight": "%FILE% ./js/modules/image-gallery.js ~ aria-hidden", | ||
| "notes": "Ensure only one active thumbnail display on slide other gets hide and add aira-hidden" |
There was a problem hiding this comment.
mispelled aria-hidden :-)
| { | ||
| "label": "Create JavaScript that ensure active image get update when any changes occurs", | ||
| "highlight": "%FILE% ./js/modules/image-gallery.js ~ this.showSlide", | ||
| "notes": "This function add shows active image in gallery and updates gallery alert message." |
There was a problem hiding this comment.
Please also highlight the update to the aria-live region in this step.
No description provided.