remove stray console log

microsoft · Feb 12, 2025 · 0b069d6 · 0b069d6
1 parent fea8cec
commit 0b069d6
Show file tree

Hide file tree

Showing 11 changed files with 110 additions and 115 deletions.
diff --git a/change/@fluentui-react-carousel-bf44df36-9898-4ce0-b4d5-decf3c170b35.json b/change/@fluentui-react-carousel-bf44df36-9898-4ce0-b4d5-decf3c170b35.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix: autoplay carousel does not trigger live region announcements",
+  "packageName": "@fluentui/react-carousel",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}
diff --git a/packages/react-components/react-carousel/library/src/components/Carousel/useCarousel.ts b/packages/react-components/react-carousel/library/src/components/Carousel/useCarousel.ts
@@ -96,7 +96,6 @@ export function useCarousel_unstable(props: CarouselProps, ref: React.Ref<HTMLDi
 
     if (announcementText !== announcementTextRef.current) {
       announcementTextRef.current = announcementText;
-      console.log('announcing', announcementText);
       announce(announcementText, { polite: true });
     }
   });

diff --git a/packages/react-components/react-dialog/stories/src/Dialog/DialogDefault.stories.tsx b/packages/react-components/react-dialog/stories/src/Dialog/DialogDefault.stories.tsx
@@ -12,26 +12,32 @@ import {
 
 export const Default = () => {
   return (
-    <Dialog>
-      <DialogTrigger disableButtonEnhancement>
-        <Button>Open dialog</Button>
-      </DialogTrigger>
-      <DialogSurface>
-        <DialogBody>
-          <DialogTitle>Dialog title</DialogTitle>
-          <DialogContent>
-            Lorem ipsum dolor sit amet consectetur adipisicing elit. Quisquam exercitationem cumque repellendus eaque
-            est dolor eius expedita nulla ullam? Tenetur reprehenderit aut voluptatum impedit voluptates in natus iure
-            cumque eaque?
-          </DialogContent>
-          <DialogActions>
-            <Button appearance="primary">Do Something</Button>
-            <DialogTrigger disableButtonEnhancement>
-              <Button appearance="secondary">Close</Button>
-            </DialogTrigger>
-          </DialogActions>
-        </DialogBody>
-      </DialogSurface>
-    </Dialog>
+    <>
+      <Dialog>
+        <DialogTrigger disableButtonEnhancement>
+          <Button>Open dialog</Button>
+        </DialogTrigger>
+        <DialogSurface>
+          <DialogBody>
+            <DialogTitle>Dialog title</DialogTitle>
+            <DialogContent>
+              Lorem ipsum dolor sit amet consectetur adipisicing elit. Quisquam exercitationem cumque repellendus eaque
+              est dolor eius expedita nulla ullam? Tenetur reprehenderit aut voluptatum impedit voluptates in natus iure
+              cumque eaque?
+            </DialogContent>
+            <DialogActions>
+              <Button appearance="primary">Do Something</Button>
+              <DialogTrigger disableButtonEnhancement>
+                <Button appearance="secondary">Close</Button>
+              </DialogTrigger>
+            </DialogActions>
+          </DialogBody>
+        </DialogSurface>
+      </Dialog>
+      <div role="listbox">
+        <span tabIndex={0}></span>
+        <button role="option">etc.</button>
+      </div>
+    </>
   );
 };
diff --git a/packages/react-components/react-menu/stories/src/Menu/MenuNestedSubmenus.stories.tsx b/packages/react-components/react-menu/stories/src/Menu/MenuNestedSubmenus.stories.tsx
@@ -4,7 +4,7 @@ import { Button, Menu, MenuTrigger, MenuList, MenuItem, MenuPopover } from '@flu
 
 const EditorLayoutSubMenu = () => {
   return (
-    <Menu>
+    <Menu positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Editor Layout</MenuItem>
       </MenuTrigger>
@@ -22,7 +22,7 @@ const EditorLayoutSubMenu = () => {
 
 const AppearanceSubMenu = () => {
   return (
-    <Menu>
+    <Menu positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Appearance</MenuItem>
       </MenuTrigger>
@@ -41,7 +41,7 @@ const AppearanceSubMenu = () => {
 
 const PreferencesSubMenu = () => {
   return (
-    <Menu>
+    <Menu positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Preferences</MenuItem>
       </MenuTrigger>
@@ -61,7 +61,7 @@ const PreferencesSubMenu = () => {
 
 export const NestedSubmenus = () => {
   return (
-    <Menu>
+    <Menu positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <Button>Toggle menu</Button>
       </MenuTrigger>

diff --git a/...ges/react-components/react-menu/stories/src/Menu/MenuNestedSubmenusControlled.stories.tsx b/...ges/react-components/react-menu/stories/src/Menu/MenuNestedSubmenusControlled.stories.tsx
@@ -10,7 +10,7 @@ const EditorLayoutSubMenu = () => {
   };
 
   return (
-    <Menu open={open} onOpenChange={onOpenChange}>
+    <Menu open={open} onOpenChange={onOpenChange} positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Editor Layout</MenuItem>
       </MenuTrigger>
@@ -33,7 +33,7 @@ const AppearanceSubMenu = () => {
   };
 
   return (
-    <Menu open={open} onOpenChange={onOpenChange}>
+    <Menu open={open} onOpenChange={onOpenChange} positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Appearance</MenuItem>
       </MenuTrigger>
@@ -57,7 +57,7 @@ const PreferencesSubMenu = () => {
   };
 
   return (
-    <Menu open={open} onOpenChange={onOpenChange}>
+    <Menu open={open} onOpenChange={onOpenChange} positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <MenuItem>Preferences</MenuItem>
       </MenuTrigger>
@@ -77,7 +77,7 @@ const PreferencesSubMenu = () => {
 
 export const NestedSubmenusControlled = () => {
   return (
-    <Menu>
+    <Menu positioning={{ autoSize: true }}>
       <MenuTrigger disableButtonEnhancement>
         <Button>Toggle menu</Button>
       </MenuTrigger>

diff --git a/...act-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx b/...act-teaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.tsx
@@ -1,33 +1,19 @@
 import * as React from 'react';
-import {
-  isHTMLElement,
-  useMergedRefs,
-  useControllableState,
-  type EventHandler,
-  useEventCallback,
-} from '@fluentui/react-utilities';
-import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
+import { isHTMLElement, useMergedRefs, useControllableState, useEventCallback } from '@fluentui/react-utilities';
+import { useAnnounce, useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts';
 
 import { CAROUSEL_ITEM } from './constants';
 import { useCarouselWalker_unstable } from './useCarouselWalker';
 import { createCarouselStore } from './createCarouselStore';
-import type { CarouselValueChangeData } from './Carousel.types';
+import type { UseCarouselOptions } from './Carousel.types';
 import { CarouselContextValue } from './CarouselContext';
 
-export type UseCarouselOptions = {
-  defaultValue?: string;
-  value?: string;
-
-  onValueChange?: EventHandler<CarouselValueChangeData>;
-  onFinish?: EventHandler<CarouselValueChangeData>;
-};
-
 // TODO: Migrate this into an external @fluentui/carousel component
 // For now, we won't export this publicly, is only for internal TeachingPopover use until stabilized.
 export function useCarousel_unstable(options: UseCarouselOptions) {
   'use no memo';
 
-  const { onValueChange, onFinish } = options;
+  const { announcement, onValueChange, onFinish } = options;
 
   const { targetDocument } = useFluent();
   const win = targetDocument?.defaultView;
@@ -41,6 +27,8 @@ export function useCarousel_unstable(options: UseCarouselOptions) {
   });
   const rootRef = React.useRef<HTMLDivElement>(null);
 
+  const { announce } = useAnnounce();
+
   if (process.env.NODE_ENV !== 'production') {
     // eslint-disable-next-line react-hooks/rules-of-hooks
     React.useEffect(() => {
@@ -115,6 +103,19 @@ export function useCarousel_unstable(options: UseCarouselOptions) {
     };
   }, [carouselWalker, store, win]);
 
+  const updateSlide = useEventCallback(
+    (event: React.MouseEvent<HTMLButtonElement | HTMLAnchorElement>, newValue: string) => {
+      setValue(newValue);
+      onValueChange?.(event, { event, type: 'click', value: newValue });
+
+      const announceText = announcement?.(newValue);
+      if (announceText) {
+        console.log('announcing', announceText);
+        announce(announceText, { polite: true });
+      }
+    },
+  );
+
   const selectPageByDirection: CarouselContextValue['selectPageByDirection'] = useEventCallback((event, direction) => {
     const active = carouselWalker.active();
 
@@ -126,25 +127,19 @@ export function useCarousel_unstable(options: UseCarouselOptions) {
       direction === 'prev' ? carouselWalker.prevPage(active.value) : carouselWalker.nextPage(active.value);
 
     if (newPage) {
-      setValue(newPage?.value);
-      onValueChange?.(event, { event, type: 'click', value: newPage?.value });
+      updateSlide(event, newPage?.value);
     } else {
       onFinish?.(event, { event, type: 'click', value: active?.value });
     }
   });
 
-  const selectPageByValue: CarouselContextValue['selectPageByValue'] = useEventCallback((event, _value) => {
-    setValue(_value);
-    onValueChange?.(event, { event, type: 'click', value: _value });
-  });
-
   return {
     carouselRef: useMergedRefs(rootRef, carouselRef),
     carousel: {
       store,
       value,
       selectPageByDirection,
-      selectPageByValue,
+      selectPageByValue: updateSlide,
     },
   };
 }
diff --git a/...eaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.types.ts b/...eaching-popover/library/src/components/TeachingPopoverCarousel/Carousel/Carousel.types.ts
@@ -1,5 +1,5 @@
 import * as React from 'react';
-import { EventData } from '@fluentui/react-utilities';
+import { EventData, EventHandler } from '@fluentui/react-utilities';
 
 export type CarouselStore = {
   clear: () => void;
@@ -15,6 +15,34 @@ export type CarouselItem = {
   value: string | null;
 };
 
+export type UseCarouselOptions = {
+  /**
+   * Localizes the string used to announce carousel page changes to screen reader users
+   * Defaults to: undefined
+   */
+  announcement?: (newValue: string) => string;
+
+  /**
+   * The initial page to display in uncontrolled mode.
+   */
+  defaultValue?: string;
+
+  /**
+   * The value of the currently active page.
+   */
+  value?: string;
+
+  /**
+   * Callback to notify a page change.
+   */
+  onValueChange?: EventHandler<CarouselValueChangeData>;
+
+  /**
+   * Callback to notify when the final button step of a carousel has been activated.
+   */
+  onFinish?: EventHandler<CarouselValueChangeData>;
+};
+
 export type CarouselValueChangeData = EventData<'click', React.MouseEvent<HTMLButtonElement | HTMLAnchorElement>> & {
   /**
    * The value to be set after event has occurred.

diff --git a/...g-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.types.ts b/...g-popover/library/src/components/TeachingPopoverCarousel/TeachingPopoverCarousel.types.ts
@@ -1,8 +1,8 @@
-import type { ComponentProps, ComponentState, EventHandler, Slot } from '@fluentui/react-utilities';
+import type { ComponentProps, ComponentState, Slot } from '@fluentui/react-utilities';
 import { type PopoverContextValue } from '@fluentui/react-popover';
 
 import { type CarouselContextValue } from './Carousel/CarouselContext';
-import type { CarouselValueChangeData } from './Carousel/Carousel.types';
+import type { UseCarouselOptions } from './Carousel/Carousel.types';
 
 export type TeachingPopoverCarouselSlots = {
   /**
@@ -14,27 +14,7 @@ export type TeachingPopoverCarouselSlots = {
 /**
  * TeachingPopoverCarousel Props
  */
-export type TeachingPopoverCarouselProps = ComponentProps<TeachingPopoverCarouselSlots> & {
-  /**
-   * The initial page to display in uncontrolled mode.
-   */
-  defaultValue?: string;
-
-  /**
-   * The value of the currently active page.
-   */
-  value?: string;
-
-  /**
-   * Callback to notify a page change.
-   */
-  onValueChange?: EventHandler<CarouselValueChangeData>;
-
-  /**
-   * Callback to notify when the final button step of a carousel has been activated.
-   */
-  onFinish?: EventHandler<CarouselValueChangeData>;
-};
+export type TeachingPopoverCarouselProps = ComponentProps<TeachingPopoverCarouselSlots> & UseCarouselOptions;
 
 /**
  * TeachingPopoverCarousel State and Context Hooks

diff --git a/...hing-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarousel.ts b/...hing-popover/library/src/components/TeachingPopoverCarousel/useTeachingPopoverCarousel.ts
@@ -2,19 +2,20 @@ import * as React from 'react';
 import { getIntrinsicElementProps, slot, useEventCallback, useMergedRefs } from '@fluentui/react-utilities';
 import type { TeachingPopoverCarouselProps, TeachingPopoverCarouselState } from './TeachingPopoverCarousel.types';
 import { usePopoverContext_unstable } from '@fluentui/react-popover';
-import { useCarousel_unstable, type UseCarouselOptions } from './Carousel/Carousel';
+import { useCarousel_unstable } from './Carousel/Carousel';
 
 export const useTeachingPopoverCarousel_unstable = (
   props: TeachingPopoverCarouselProps,
   ref: React.Ref<HTMLDivElement>,
 ): TeachingPopoverCarouselState => {
   const toggleOpen = usePopoverContext_unstable(c => c.toggleOpen);
-  const handleFinish: UseCarouselOptions['onFinish'] = useEventCallback((event, data) => {
+  const handleFinish: TeachingPopoverCarouselProps['onFinish'] = useEventCallback((event, data) => {
     props.onFinish?.(event, data);
     toggleOpen(event as React.MouseEvent<HTMLElement>);
   });
 
   const { carousel, carouselRef } = useCarousel_unstable({
+    announcement: props.announcement,
     defaultValue: props.defaultValue,
     value: props.value,
     onValueChange: props.onValueChange,

diff --git a/...ts/react-teaching-popover/stories/src/TeachingPopover/TeachingPopoverCarousel.stories.tsx b/...ts/react-teaching-popover/stories/src/TeachingPopover/TeachingPopoverCarousel.stories.tsx
@@ -17,14 +17,18 @@ import {
 
 const swapImage = 'https://fabricweb.azureedge.net/fabric-website/assets/images/wireframe/image-square.png';
 
+const getAnnouncement = (newValue: string) => {
+  return `Carousel slide ${newValue}`;
+};
+
 export const Carousel = () => (
   <TeachingPopover>
     <TeachingPopoverTrigger>
       <Button>TeachingPopover trigger</Button>
     </TeachingPopoverTrigger>
     <TeachingPopoverSurface>
       <TeachingPopoverHeader>Tips</TeachingPopoverHeader>
-      <TeachingPopoverCarousel defaultValue={'1'}>
+      <TeachingPopoverCarousel defaultValue={'1'} announcement={getAnnouncement}>
         <TeachingPopoverCarouselCard value="1">
           <TeachingPopoverBody media={<Image alt="test image" fit="cover" src={swapImage} />}>
             <TeachingPopoverTitle>Teaching Bubble Title</TeachingPopoverTitle>