♿ Update amp-carousel focus styles for better color contrast (#29564) (…

…#30750) * Update amp-carousel focus styles for better color contrast (#29564) * Add unit tests to verify focus style updates * Removed console.log
ampproject · Oct 26, 2020 · 131d74b · 131d74b
1 parent d140ea4
commit 131d74b
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 1 deletion.
diff --git a/extensions/amp-carousel/0.1/amp-carousel.css b/extensions/amp-carousel/0.1/amp-carousel.css
@@ -47,6 +47,11 @@ amp-carousel {
   z-index: 10;
 }
 
+.amp-carousel-button:focus {
+  border: 1px black solid;
+  outline: 1px white solid;
+}
+
 amp-carousel[controls] .amp-carousel-button,
 amp-carousel.i-amphtml-carousel-has-controls .amp-carousel-button,
 .amp-mode-mouse .amp-carousel-button {

diff --git a/extensions/amp-carousel/0.1/test/test-slidescroll.js b/extensions/amp-carousel/0.1/test/test-slidescroll.js
@@ -447,6 +447,31 @@ describes.realWin(
       });
     });
 
+    it('should show focus outline and border on next and prev buttons', () => {
+      return getAmpSlideScroll().then((ampSlideScroll) => {
+        const impl = ampSlideScroll.implementation_;
+        impl.showSlide_(1);
+
+        impl.prevButton_.focus();
+        expect(doc.activeElement).to.equal(impl.prevButton_);
+        expect(win.getComputedStyle(impl.prevButton_).outline).to.equal(
+          'rgb(255, 255, 255) solid 1px'
+        );
+        expect(win.getComputedStyle(impl.prevButton_).border).to.equal(
+          '1px solid rgb(0, 0, 0)'
+        );
+
+        impl.nextButton_.focus();
+        expect(doc.activeElement).to.equal(impl.nextButton_);
+        expect(win.getComputedStyle(impl.nextButton_).outline).to.equal(
+          'rgb(255, 255, 255) solid 1px'
+        );
+        expect(win.getComputedStyle(impl.nextButton_).border).to.equal(
+          '1px solid rgb(0, 0, 0)'
+        );
+      });
+    });
+
     it('should set the correct scrollLeft when there is only one slide', () => {
       return getAmpSlideScroll().then((ampSlideScroll) => {
         const impl = ampSlideScroll.implementation_;

diff --git a/extensions/amp-carousel/0.2/amp-carousel.css b/extensions/amp-carousel/0.2/amp-carousel.css
@@ -93,6 +93,11 @@ amp-carousel:not([type="slides"]) .i-amphtml-carousel-content {
   pointer-events: all;
 }
 
+.amp-carousel-button:focus {
+  border: 1px black solid;
+  outline: 1px white solid;
+}
+
 .amp-carousel-button.amp-disabled {
   animation: none;
   opacity: 0;

diff --git a/extensions/amp-carousel/0.2/test/test-type-slides.js b/extensions/amp-carousel/0.2/test/test-type-slides.js
@@ -27,7 +27,7 @@ import {getDetail, listenOncePromise} from '../../../../src/event-helper';
 
 /**
  * @param {!Element} el
- * @param {number=} index An intex to wait for.
+ * @param {number=} index An index to wait for.
  * @return {!Promise<undefined>}
  */
 async function afterIndexUpdate(el, index) {
@@ -156,6 +156,32 @@ describes.realWin(
       expect(slideWrappers[1].getAttribute('aria-hidden')).to.equal('true');
     });
 
+    it('should show focus outline and border on next and prev buttons', async () => {
+      const carousel = await getCarousel({loop: false});
+
+      carousel.implementation_.interactionNext();
+      await afterIndexUpdate(carousel);
+
+      const impl = carousel.implementation_;
+      impl.prevButton_.focus();
+      expect(doc.activeElement).to.equal(impl.prevButton_);
+      expect(win.getComputedStyle(impl.prevButton_).outline).to.equal(
+        'rgb(255, 255, 255) solid 1px'
+      );
+      expect(win.getComputedStyle(impl.prevButton_).border).to.equal(
+        '1px solid rgb(0, 0, 0)'
+      );
+
+      impl.nextButton_.focus();
+      expect(doc.activeElement).to.equal(impl.nextButton_);
+      expect(win.getComputedStyle(impl.nextButton_).outline).to.equal(
+        'rgb(255, 255, 255) solid 1px'
+      );
+      expect(win.getComputedStyle(impl.nextButton_).border).to.equal(
+        '1px solid rgb(0, 0, 0)'
+      );
+    });
+
     describe('loop', () => {
       it('should go to the correct slide clicking next', async () => {
         const carousel = await getCarousel({loop: true});