fix(rating): adds focus outline on click (#7341)

**Related Issue:** #4633 , #6642 ## Summary This PR will add focus outline for `calcite-rating` component on click.
Esri · Aug 26, 2023 · af30073 · af30073
1 parent 44d2660
commit af30073
Show file tree

Hide file tree

Showing 5 changed files with 187 additions and 172 deletions.
diff --git a/packages/calcite-components/src/components/rating/interfaces.ts b/packages/calcite-components/src/components/rating/interfaces.ts
@@ -3,14 +3,13 @@ import { Scale } from "../interfaces";
 export interface Star {
   average: boolean;
   checked: boolean;
-  focused: boolean;
   fraction: number;
   hovered: boolean;
   id: string;
-  idx: number;
   partial: boolean;
   selected: boolean;
   value: number;
+  tabIndex: number;
 }
 
 export interface StarIconProps {

diff --git a/packages/calcite-components/src/components/rating/rating.e2e.ts b/packages/calcite-components/src/components/rating/rating.e2e.ts
@@ -9,6 +9,7 @@ import {
   renders,
   t9n,
 } from "../../tests/commonTests";
+import { isElementFocused } from "../../tests/utils";
 
 describe("calcite-rating", () => {
   describe("common tests", () => {
@@ -38,19 +39,19 @@ describe("calcite-rating", () => {
 
     describe("should focus input element in shadow DOM", () => {
       focusable("calcite-rating", {
-        shadowFocusTargetSelector: "input",
+        shadowFocusTargetSelector: "label",
       });
     });
 
     describe("focuses the first star when the label is clicked and no-rating value exists", () => {
       labelable("calcite-rating", {
-        shadowFocusTargetSelector: "input[value='1']",
+        shadowFocusTargetSelector: "label[data-value='1']",
       });
     });
 
     describe("focuses the value-matching star when the label is clicked", () => {
       labelable("<calcite-rating value='3'></calcite-rating>", {
-        shadowFocusTargetSelector: "input[value='3']",
+        shadowFocusTargetSelector: "label[data-value='3']",
       });
     });
 
@@ -65,17 +66,16 @@ describe("calcite-rating", () => {
       await page.setContent("<calcite-rating></calcite-rating>");
       const icons = await page.findAll("calcite-rating >>> .icon");
       const labels = await page.findAll("calcite-rating >>> .star");
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
       const element = await page.find("calcite-rating");
       const changeEvent = await element.spyOnEvent("calciteRatingChange");
 
       expect(await page.find("calcite-rating >>> .fraction")).toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).toBeNull();
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(0);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(0);
       expect(element).toEqualAttribute("value", "0");
       expect(changeEvent).toHaveReceivedEventTimes(0);
 
@@ -104,27 +104,26 @@ describe("calcite-rating", () => {
       expect(labels[2]).not.toHaveClass("partial");
       expect(labels[3]).not.toHaveClass("partial");
       expect(labels[4]).not.toHaveClass("partial");
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(0);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(0);
     });
 
     it("should render a rating with an average", async () => {
       const page = await newE2EPage();
       await page.setContent("<calcite-rating average=3.4></calcite-rating>");
       const icons = await page.findAll("calcite-rating >>> .icon");
       const labels = await page.findAll("calcite-rating >>> .star");
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
       const element = await page.find("calcite-rating");
       const changeEvent = await element.spyOnEvent("calciteRatingChange");
 
       expect(await page.find("calcite-rating >>> .fraction")).not.toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).not.toBeNull();
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(0);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(0);
       expect(element).toEqualAttribute("value", "0");
       expect(changeEvent).toHaveReceivedEventTimes(0);
 
@@ -162,17 +161,16 @@ describe("calcite-rating", () => {
       await page.setContent("<calcite-rating value=4></calcite-rating>");
       const icons = await page.findAll("calcite-rating >>> .icon");
       const labels = await page.findAll("calcite-rating >>> .star");
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
       const element = await page.find("calcite-rating");
       const changeEvent = await element.spyOnEvent("calciteRatingChange");
 
       expect(await page.find("calcite-rating >>> .fraction")).toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).toBeNull();
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(4);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(4);
       expect(element).toEqualAttribute("value", "4");
       expect(changeEvent).toHaveReceivedEventTimes(0);
 
@@ -209,17 +207,16 @@ describe("calcite-rating", () => {
       await page.setContent("<calcite-rating value=3 average=4.2></calcite-rating>");
       const icons = await page.findAll("calcite-rating >>> .icon");
       const labels = await page.findAll("calcite-rating >>> .star");
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
       const element = await page.find("calcite-rating");
       const changeEvent = await element.spyOnEvent("calciteRatingChange");
 
       expect(await page.find("calcite-rating >>> .fraction")).toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).toBeNull();
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(3);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(3);
       expect(element).toEqualAttribute("value", "3");
       expect(changeEvent).toHaveReceivedEventTimes(0);
 
@@ -362,15 +359,15 @@ describe("calcite-rating", () => {
       await labels[2].click();
       await page.waitForChanges();
 
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      const dataValue = labels[2].getAttribute("data-value");
 
       expect(await page.find("calcite-rating >>> .fraction")).toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).toBeNull();
-      expect(hoveredEl.length).toBe(3);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(3);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, `.star[data-value='${dataValue}']`, { shadowed: true })).toBe(true);
+      expect(selectedElements.length).toBe(3);
       expect(element).toEqualAttribute("value", "3");
       expect(changeEvent).toHaveReceivedEventTimes(1);
 
@@ -413,15 +410,14 @@ describe("calcite-rating", () => {
       await labels[2].hover();
       await page.waitForChanges();
 
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
 
       expect(await page.find("calcite-rating >>> .fraction")).toBeNull();
       expect(await page.find("calcite-rating >>> .partial")).toBeNull();
-      expect(hoveredEl.length).toBe(3);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(0);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(0);
       expect(element).toEqualAttribute("value", "0");
       expect(changeEvent).toHaveReceivedEventTimes(0);
 
@@ -462,13 +458,12 @@ describe("calcite-rating", () => {
       await labels[3].hover();
       await page.waitForChanges();
 
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
 
-      expect(focusedEl.length).toEqual(0);
-      expect(hoveredEl.length).toEqual(4);
-      expect(selectedEl.length).toEqual(3);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(hoveredElements.length).toEqual(4);
+      expect(selectedElements.length).toEqual(3);
       expect(icons[0]).toEqualAttribute("icon", "star-f");
       expect(icons[1]).toEqualAttribute("icon", "star-f");
       expect(icons[2]).toEqualAttribute("icon", "star-f");
@@ -504,9 +499,16 @@ describe("calcite-rating", () => {
       const changeEvent = await element.spyOnEvent("calciteRatingChange");
 
       await ratingItem1.click();
+      await page.waitForChanges();
+
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
 
       expect(element).toEqualAttribute("value", "4");
       expect(changeEvent).toHaveReceivedEventTimes(0);
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(4);
     });
 
     it("should reset the rating when the current value is equal to the value of the clicked input", async () => {
@@ -519,8 +521,15 @@ describe("calcite-rating", () => {
       await labels[2].click();
       await page.waitForChanges();
 
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      const dataValue = labels[2].getAttribute("data-value");
+
       expect(await element.getProperty("value")).toBe(0);
       expect(changeEvent).toHaveReceivedEventTimes(1);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, `.star[data-value='${dataValue}']`, { shadowed: true })).toBe(true);
+      expect(selectedElements.length).toBe(0);
     });
 
     it("should not allow rating to be cleared/reset when required props is set true", async () => {
@@ -534,8 +543,15 @@ describe("calcite-rating", () => {
       await labels[2].click();
       await page.waitForChanges();
 
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      const dataValue = labels[2].getAttribute("data-value");
+
       expect(await element.getProperty("value")).toBe(3);
       expect(changeEvent).toHaveReceivedEventTimes(1);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, `.star[data-value='${dataValue}']`, { shadowed: true })).toBe(true);
+      expect(selectedElements.length).toBe(3);
     });
 
     it("should not allow click interaction when read-only is set", async () => {
@@ -545,7 +561,15 @@ describe("calcite-rating", () => {
       const ratingItem1 = await page.find("calcite-rating >>> .star");
 
       await ratingItem1.click();
+      await page.waitForChanges();
+
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+
       expect(element).toEqualAttribute("value", "4");
+      expect(hoveredElements.length).toBe(0);
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(selectedElements.length).toBe(4);
     });
   });
 
@@ -558,31 +582,33 @@ describe("calcite-rating", () => {
       await page.keyboard.press("Tab");
       await page.waitForChanges();
 
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      const labels = await page.findAll("calcite-rating >>> .star");
+      const dataValue = labels[2].getAttribute("data-value");
 
-      expect(hoveredEl.length).toBe(3);
-      expect(focusedEl.length).toBe(1);
-      expect(selectedEl.length).toBe(3);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, `.star[data-value='${dataValue}']`, { shadowed: true })).toBe(true);
+      expect(selectedElements.length).toBe(3);
       expect(element).toEqualAttribute("value", "3");
     });
 
     it("should update the UI when the element's blur event is triggered", async () => {
       const page = await newE2EPage();
-      await page.setContent("<calcite-rating></calcite-rating>");
+      await page.setContent("<calcite-rating></calcite-rating> <calcite-button> Click</calcite-button>");
       const element = await page.find("calcite-rating");
+
       await page.keyboard.press("Tab");
+      await page.waitForChanges();
       await page.keyboard.press("Tab");
       await page.waitForChanges();
       await page.waitForTimeout(200);
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
 
-      expect(hoveredEl.length).toBe(0);
-      expect(focusedEl.length).toBe(0);
-      expect(selectedEl.length).toBe(0);
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      expect(await isElementFocused(page, "calcite-rating")).toBe(false);
+      expect(hoveredElements.length).toBe(0);
+      expect(selectedElements.length).toBe(0);
       expect(element).toEqualAttribute("value", "0");
     });
 
@@ -597,14 +623,14 @@ describe("calcite-rating", () => {
       await page.keyboard.press("Tab");
       await page.keyboard.up("Shift");
       await page.waitForChanges();
+      const hoveredElements = await page.findAll("calcite-rating >>> .star.hovered");
+      const selectedElements = await page.findAll("calcite-rating >>> .star.selected");
+      const labels = await page.findAll("calcite-rating >>> .star");
+      const dataValue = labels[2].getAttribute("data-value");
 
-      const focusedEl = await page.findAll("calcite-rating >>> .star.focused");
-      const hoveredEl = await page.findAll("calcite-rating >>> .star.hovered");
-      const selectedEl = await page.findAll("calcite-rating >>> .star.selected");
-
-      expect(hoveredEl.length).toBe(3);
-      expect(focusedEl.length).toBe(1);
-      expect(selectedEl.length).toBe(3);
+      expect(hoveredElements.length).toBe(3);
+      expect(await isElementFocused(page, `.star[data-value='${dataValue}']`, { shadowed: true })).toBe(true);
+      expect(selectedElements.length).toBe(3);
       expect(element).toEqualAttribute("value", "3");
     });
 
@@ -663,11 +689,11 @@ describe("calcite-rating", () => {
       expect(labels[2]).not.toHaveClass("selected");
       expect(labels[3]).not.toHaveClass("selected");
       expect(labels[4]).not.toHaveClass("selected");
-      expect(labels[0]).not.toHaveClass("focused");
-      expect(labels[1]).toHaveClass("focused");
-      expect(labels[2]).not.toHaveClass("focused");
-      expect(labels[3]).not.toHaveClass("focused");
-      expect(labels[4]).not.toHaveClass("focused");
+      expect(await isElementFocused(page, `[for=${labels[0].getAttribute("for")}]`, { shadowed: true })).toBe(false);
+      expect(await isElementFocused(page, `[for=${labels[1].getAttribute("for")}]`, { shadowed: true })).toBe(true);
+      expect(await isElementFocused(page, `[for=${labels[2].getAttribute("for")}]`, { shadowed: true })).toBe(false);
+      expect(await isElementFocused(page, `[for=${labels[3].getAttribute("for")}]`, { shadowed: true })).toBe(false);
+      expect(await isElementFocused(page, `[for=${labels[4].getAttribute("for")}]`, { shadowed: true })).toBe(false);
       expect(element).toEqualAttribute("value", "2");
       expect(changeEvent).toHaveReceivedEventTimes(3);
     });

diff --git a/packages/calcite-components/src/components/rating/rating.scss b/packages/calcite-components/src/components/rating/rating.scss
@@ -52,10 +52,9 @@
   flex-direction: column;
   cursor: pointer;
   color: theme("borderColor.color.input");
-}
-
-.focused {
-  @apply focus-outset;
+  &:focus {
+    @apply focus-outset;
+  }
 }
 
 .average,

diff --git a/packages/calcite-components/src/components/rating/rating.stories.ts b/packages/calcite-components/src/components/rating/rating.stories.ts
@@ -41,3 +41,15 @@ export const darkModeRTL_TestOnly = (): string => html`
 darkModeRTL_TestOnly.parameters = { modes: modesDarkDefault };
 
 export const disabled_TestOnly = (): string => html`<calcite-rating disabled value="3"></calcite-rating>`;
+
+export const Focus_TestOnly = (): string => html` <calcite-rating value="4" required></calcite-rating>
+  <script>
+    (async () => {
+      await customElements.whenDefined("calcite-rating");
+      await document.querySelector("calcite-rating").setFocus();
+    })();
+  </script>`;
+
+Focus_TestOnly.parameters = {
+  chromatic: { delay: 500 },
+};