Modify localization strings for carousel flippers (#4646)

* Change left/right to next/previous * Rename loc string ID and add tests * Fix tests
microsoft · Feb 16, 2023 · fa77780 · fa77780
1 parent 30b6b3c
commit fa77780
Show file tree

Hide file tree

Showing 12 changed files with 163 additions and 18 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 -  Resolved [#4643](https://github.com/microsoft/BotFramework-WebChat/issues/4643). Decoupling `botframework-directlinejs` from business logic of Web Chat for better tree-shaking, by [@compulim](https://github.com/compulim), in PR [#4645](https://github.com/microsoft/BotFramework-WebChat/pull/4645)
 
+### Fixed
+
+-  Fixes [#4557](https://github.com/microsoft/BotFramework-WebChat/issues/4557). Flipper buttons in carousels and suggested actions is now renamed to "next/previous" from "left/right", by [@compulim](https://github.com/compulim), in PR [#4646](https://github.com/microsoft/BotFramework-WebChat/pull/4646)
+
 ## [4.15.7] - 2023-02-15
 
 ### Added

diff --git a/__tests__/carousel.js b/__tests__/carousel.js
@@ -23,7 +23,7 @@ describe('carousel without avatar initials', () => {
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 
-    const rightFlipper = await driver.findElement(By.css('button[aria-label="Right"]'));
+    const rightFlipper = await driver.findElement(By.css('button[aria-label="Next"]'));
 
     await rightFlipper.click();
     await rightFlipper.click();
@@ -46,7 +46,7 @@ describe('carousel without avatar initials', () => {
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 
-    const rightFlipper = await driver.findElement(By.css('button[aria-label="Right"]'));
+    const rightFlipper = await driver.findElement(By.css('button[aria-label="Next"]'));
 
     await rightFlipper.click();
     await rightFlipper.click();
@@ -121,7 +121,7 @@ describe('carousel with avatar initials', () => {
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 
-    const rightFlipper = await driver.findElement(By.css('button[aria-label="Right"]'));
+    const rightFlipper = await driver.findElement(By.css('button[aria-label="Next"]'));
 
     await rightFlipper.click();
     await rightFlipper.click();
@@ -144,7 +144,7 @@ describe('carousel with avatar initials', () => {
 
     expect(await driver.takeScreenshot()).toMatchImageSnapshot(imageSnapshotOptions);
 
-    const rightFlipper = await driver.findElement(By.css('button[aria-label="Right"]'));
+    const rightFlipper = await driver.findElement(By.css('button[aria-label="Next"]'));
 
     await rightFlipper.click();
     await rightFlipper.click();

diff --git a/__tests__/html/carousel.flipperButton.html b/__tests__/html/carousel.flipperButton.html
@@ -29,7 +29,7 @@
         expect(carouselFilmstrip.scrollLeft).toBe(0);
 
         // WHEN: Right flipper is clicked.
-        const rightFlipper = carouselLayout.querySelector('[aria-label="Right"]');
+        const rightFlipper = carouselLayout.querySelector('[aria-label="Next"]');
 
         // Improve test reliability by hover before click on flipper button.
         await host.hover(rightFlipper);
@@ -43,7 +43,7 @@
         await host.snapshot();
 
         // WHEN: Left flipper is clicked.
-        const leftFlipper = carouselLayout.querySelector('[aria-label="Left"]');
+        const leftFlipper = carouselLayout.querySelector('[aria-label="Previous"]');
 
         // Improve test reliability by hover before click on flipper button.
         await host.hover(leftFlipper);

diff --git a/__tests__/html/carousel.flipperButton.rtl.html b/__tests__/html/carousel.flipperButton.rtl.html
@@ -29,8 +29,8 @@
 
         expect(carouselFilmstrip.scrollLeft).toBe(0);
 
-        // WHEN: Left flipper is clicked.
-        const leftFlipper = carouselLayout.querySelector('[aria-label="Left"]');
+        // WHEN: Left flipper is clicked. In RTL, the left flipper goes next.
+        const leftFlipper = carouselLayout.querySelector('[aria-label="Next"]');
 
         // Improve test reliability by hover before click on flipper button.
         await host.hover(leftFlipper);
@@ -43,8 +43,8 @@
         await testHelpers.sleep(500); // Wait both flippers to fade in.
         await host.snapshot();
 
-        // WHEN: Right flipper is clicked.
-        const rightFlipper = carouselLayout.querySelector('[aria-label="Right"]');
+        // WHEN: Right flipper is clicked. In RTL, the right flipper goes back.
+        const rightFlipper = carouselLayout.querySelector('[aria-label="Previous"]');
 
         // Improve test reliability by hover before click on flipper button.
         await host.hover(rightFlipper);

diff --git a/__tests__/html/suggestedActions.scroll.html b/__tests__/html/suggestedActions.scroll.html
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html lang="en-US">
+  <head>
+    <link href="/assets/index.css" rel="stylesheet" type="text/css" />
+    <script crossorigin="anonymous" src="/test-harness.js"></script>
+    <script crossorigin="anonymous" src="/test-page-object.js"></script>
+    <script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
+  </head>
+  <body>
+    <div id="webchat"></div>
+    <script>
+      run(async function () {
+        WebChat.renderWebChat(
+          {
+            directLine: WebChat.createDirectLine({ token: await testHelpers.token.fetchDirectLineToken() }),
+            store: testHelpers.createStore()
+          },
+          document.getElementById('webchat')
+        );
+
+        await pageConditions.uiConnected();
+
+        // GIVEN: Show the suggested actions.
+        await pageObjects.sendMessageViaSendBox('suggested-actions');
+        await pageConditions.numActivitiesShown(2);
+
+        // WHEN: The right flipper button is clicked.
+        document.querySelector('[aria-label="Suggested actions"] [aria-label="right"]').click();
+
+        // TODO: This will be updated from "left/right" to "next/previous" when the carousel support customizing the aria-label. Commented out for now.
+        // document.querySelector('[aria-label="Suggested actions"] [aria-label="next"]').click();
+
+        // THEN: It should scroll to the right.
+        await pageConditions.became(
+          'suggested actions scrolled to the right',
+          () => document.querySelector('.react-film__filmstrip').scrollLeft > 0,
+          1000
+        );
+
+        // WHEN: The left flipper button is clicked.
+        document.querySelector('[aria-label="Suggested actions"] [aria-label="left"]').click();
+
+        // TODO: This will be updated from "left/right" to "next/previous" when the carousel support customizing the aria-label. Commented out for now.
+        // document.querySelector('[aria-label="Suggested actions"] [aria-label="previous"]').click();
+
+        // THEN: It should scroll back to the origin.
+        await pageConditions.became(
+          'suggested actions scrolled back to the origin',
+          () => document.querySelector('.react-film__filmstrip').scrollLeft === 0,
+          1000
+        );
+      });
+    </script>
+  </body>
+</html>
diff --git a/__tests__/html/suggestedActions.scroll.js b/__tests__/html/suggestedActions.scroll.js
@@ -0,0 +1,5 @@
+/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */
+
+describe('suggested actions', () => {
+  test('should scroll when flipper buttons are clicked', () => runHTML('suggestedActions.scroll.html'));
+});
diff --git a/__tests__/html/suggestedActions.scroll.rtl.html b/__tests__/html/suggestedActions.scroll.rtl.html
@@ -0,0 +1,56 @@
+<!DOCTYPE html>
+<html lang="en-US">
+  <head>
+    <link href="/assets/index.css" rel="stylesheet" type="text/css" />
+    <script crossorigin="anonymous" src="/test-harness.js"></script>
+    <script crossorigin="anonymous" src="/test-page-object.js"></script>
+    <script crossorigin="anonymous" src="/__dist__/webchat-es5.js"></script>
+  </head>
+  <body>
+    <div id="webchat"></div>
+    <script>
+      run(async function () {
+        WebChat.renderWebChat(
+          {
+            dir: 'rtl',
+            directLine: WebChat.createDirectLine({ token: await testHelpers.token.fetchDirectLineToken() }),
+            store: testHelpers.createStore()
+          },
+          document.getElementById('webchat')
+        );
+
+        await pageConditions.uiConnected();
+
+        // GIVEN: Show the suggested actions.
+        await pageObjects.sendMessageViaSendBox('suggested-actions');
+        await pageConditions.numActivitiesShown(2);
+
+        // WHEN: The right flipper button is clicked.
+        document.querySelector('[aria-label="Suggested actions"] [aria-label="left"]').click();
+
+        // TODO: This will be updated from "left/right" to "next/previous" when the carousel support customizing the aria-label. Commented out for now.
+        // document.querySelector('[aria-label="Suggested actions"] [aria-label="next"]').click();
+
+        // THEN: It should scroll to the right.
+        await pageConditions.became(
+          'suggested actions scrolled to the left',
+          () => document.querySelector('.react-film__filmstrip').scrollLeft < 0,
+          1000
+        );
+
+        // WHEN: The left flipper button is clicked.
+        document.querySelector('[aria-label="Suggested actions"] [aria-label="right"]').click();
+
+        // TODO: This will be updated from "left/right" to "next/previous" when the carousel support customizing the aria-label. Commented out for now.
+        // document.querySelector('[aria-label="Suggested actions"] [aria-label="previous"]').click();
+
+        // THEN: It should scroll back to the origin.
+        await pageConditions.became(
+          'suggested actions scrolled back to the origin',
+          () => document.querySelector('.react-film__filmstrip').scrollLeft === 0,
+          1000
+        );
+      });
+    </script>
+  </body>
+</html>
diff --git a/__tests__/html/suggestedActions.scroll.rtl.js b/__tests__/html/suggestedActions.scroll.rtl.js
@@ -0,0 +1,6 @@
+/** @jest-environment ./packages/test/harness/src/host/jest/WebDriverEnvironment.js */
+
+describe('In RTL', () =>
+  describe('suggested actions', () => {
+    test('should scroll when flipper buttons are clicked', () => runHTML('suggestedActions.scroll.rtl.html'));
+  }));
diff --git a/packages/api/src/localization/en-US.json b/packages/api/src/localization/en-US.json
@@ -52,10 +52,10 @@
   "ATTACHMENT_TEXT": "A text: $1",
   "_ATTACHMENT_TEXT.comment": "$1 is the content.",
   "ATTACHMENT_VIDEO": "A video clip",
-  "CAROUSEL_FLIPPER_LEFT_ALT": "Left",
-  "_CAROUSEL_FLIPPER_LEFT_ALT.comment": "This is for screen reader for the label of the left flipper button for carousels.",
-  "CAROUSEL_FLIPPER_RIGHT_ALT": "Right",
-  "_CAROUSEL_FLIPPER_RIGHT_ALT.comment": "This is for screen reader for the label of the right flipper button for carousels.",
+  "CAROUSEL_FLIPPER_NEXT_ALT": "Next",
+  "_CAROUSEL_FLIPPER_NEXT_ALT.comment": "This is for screen reader for the label of the right flipper button for carousels for left-to-right language or left flipper button for right-to-left (RTL) languages. To support RTL, the language used in this text should be less directional.",
+  "CAROUSEL_FLIPPER_PREVIOUS_ALT": "Previous",
+  "_CAROUSEL_FLIPPER_PREVIOUS_ALT.comment": "This is for screen reader for the label of the left flipper button for carousels for left-to-right language or right flipper button for right-to-left (RTL) languages. To support RTL, the language used in this text should be less directional.",
   "CONNECTIVITY_STATUS_ALT_CONNECTED": "Connected",
   "CONNECTIVITY_STATUS_ALT_CONNECTING": "Connecting…",
   "CONNECTIVITY_STATUS_ALT_FATAL": "Unable to connect.",
@@ -99,6 +99,10 @@
   "SPEECH_INPUT_MICROPHONE_BUTTON_OPEN_ALT": "Microphone on",
   "_SPEECH_INPUT_MICROPHONE_BUTTON_OPEN_ALT.comment": "This is for screen reader and is the label of the microphone button, when clicked, will open microphone.",
   "SPEECH_INPUT_STARTING": "Starting…",
+  "SUGGESTED_ACTIONS_FLIPPER_NEXT_ALT": "Next",
+  "_SUGGESTED_ACTIONS_FLIPPER_NEXT_ALT.comment": "This is for screen reader for the label of the right flipper button for suggested actions. Probably can re-use the value from CAROUSEL_FLIPPER_NEXT_ALT.",
+  "SUGGESTED_ACTIONS_FLIPPER_PREVIOUS_ALT": "Previous",
+  "_SUGGESTED_ACTIONS_FLIPPER_PREVIOUS_ALT.comment": "This is for screen reader for the label of the left flipper button for suggested actions. Probably can re-use the value from CAROUSEL_FLIPPER_PREVIOUS_ALT.",
   "SUGGESTED_ACTIONS_LABEL_ALT": "Suggested actions",
   "_SUGGESTED_ACTIONS_LABEL_ALT.comment": "This is for screen reader. Browser will read as \"Suggested actions toolbar\", where \"toolbar\" is a role name appended by browser.",
   "TEXT_INPUT_ALT": "Message input box",

diff --git a/packages/api/src/localization/yue.json b/packages/api/src/localization/yue.json
@@ -32,8 +32,8 @@
   "ATTACHMENT_IMAGE": "一幅圖",
   "ATTACHMENT_TEXT": "一段文字：$1",
   "ATTACHMENT_VIDEO": "一段錄像",
-  "CAROUSEL_FLIPPER_LEFT_ALT": "左",
-  "CAROUSEL_FLIPPER_RIGHT_ALT": "右",
+  "CAROUSEL_FLIPPER_NEXT_ALT": "後",
+  "CAROUSEL_FLIPPER_PREVIOUS_ALT": "前",
   "CONNECTIVITY_STATUS_ALT_CONNECTED": "接駁到",
   "CONNECTIVITY_STATUS_ALT_CONNECTING": "接駁緊…",
   "CONNECTIVITY_STATUS_ALT_FATAL": "接駁唔倒。",
@@ -70,6 +70,8 @@
   "SPEECH_INPUT_MICROPHONE_BUTTON_CLOSE_ALT": "閂咪",
   "SPEECH_INPUT_MICROPHONE_BUTTON_OPEN_ALT": "開咪",
   "SPEECH_INPUT_STARTING": "開始緊…",
+  "SUGGESTED_ACTIONS_FLIPPER_NEXT_ALT": "後",
+  "SUGGESTED_ACTIONS_FLIPPER_PREVIOUS_ALT": "前",
   "SUGGESTED_ACTIONS_LABEL_ALT": "建議㩒嘅掣",
   "TEXT_INPUT_ALT": "訊息輸入盒",
   "TEXT_INPUT_PLACEHOLDER": "輸入你嘅訊息",

diff --git a/packages/component/src/Activity/CarouselLayout.js b/packages/component/src/Activity/CarouselLayout.js
@@ -44,6 +44,12 @@ const CarouselLayoutCore = ({
   const rightSideFlipper = direction === 'rtl' ? '<' : '>';
   const rootClassName = useStyleToEmotionObject()(ROOT_STYLE) + '';
 
+  const nextAlt = localize('CAROUSEL_FLIPPER_NEXT_ALT');
+  const previousAlt = localize('CAROUSEL_FLIPPER_PREVIOUS_ALT');
+
+  const leftFlipperAriaLabel = direction === 'rtl' ? nextAlt : previousAlt;
+  const rightFlipperAriaLabel = direction === 'rtl' ? previousAlt : nextAlt;
+
   return (
     <div
       className={classNames('webchat__carousel-layout', rootClassName, carouselFlipperStyleSet + '', filmRootClassName)}
@@ -59,10 +65,10 @@ const CarouselLayoutCore = ({
         />
         {scrollBarWidth !== '100%' && (
           <React.Fragment>
-            <Flipper aria-label={localize('CAROUSEL_FLIPPER_LEFT_ALT')} blurFocusOnClick={true} mode="left">
+            <Flipper aria-label={leftFlipperAriaLabel} blurFocusOnClick={true} mode="left">
               {leftSideFlipper}
             </Flipper>
-            <Flipper aria-label={localize('CAROUSEL_FLIPPER_RIGHT_ALT')} blurFocusOnClick={true} mode="right">
+            <Flipper aria-label={rightFlipperAriaLabel} blurFocusOnClick={true} mode="right">
               {rightSideFlipper}
             </Flipper>
           </React.Fragment>

diff --git a/packages/component/src/SendBox/SuggestedActions.tsx b/packages/component/src/SendBox/SuggestedActions.tsx
@@ -53,6 +53,7 @@ const SuggestedActionCarouselContainer = ({ children, className, label }) => {
   const [{ suggestedActions: suggestedActionsStyleSet }] = useStyleSet();
   const [direction] = useDirection();
   const [nonce] = useNonce();
+  const localize = useLocalizer();
   const ref = useRef();
   const rootClassName = useStyleToEmotionObject()(ROOT_STYLE) + '';
 
@@ -70,8 +71,12 @@ const SuggestedActionCarouselContainer = ({ children, className, label }) => {
       suggestedActionsCarouselFlipperSize
     ]
   );
+  const nextFlipperAriaLabel = localize('SUGGESTED_ACTIONS_FLIPPER_NEXT_ALT');
+  const previousFlipperAriaLabel = localize('SUGGESTED_ACTIONS_FLIPPER_PREVIOUS_ALT');
 
   const [focusedWithin] = useFocusWithin(ref);
+  const leftFlipperAriaLabel = direction === 'rtl' ? nextFlipperAriaLabel : previousFlipperAriaLabel;
+  const rightFlipperAriaLabel = direction === 'rtl' ? previousFlipperAriaLabel : nextFlipperAriaLabel;
 
   return (
     // TODO: The content of suggested actions should be the labelled by the activity.
@@ -99,7 +104,9 @@ const SuggestedActionCarouselContainer = ({ children, className, label }) => {
           className="webchat__suggested-actions__carousel"
           dir={direction}
           flipperBlurFocusOnClick={true}
+          leftFlipperAriaLabel={leftFlipperAriaLabel}
           nonce={nonce}
+          rightFlipperAriaLabel={rightFlipperAriaLabel}
           showDots={false}
           showScrollBar={false}
           styleSet={filmStyleSet}