[⏪ Reverted] fix(PageLayout): Remove warning for the deprecated position prop

e2e/components/ToggleSwitch.test.ts

@@ -30,11 +30,6 @@ test.describe('ToggleSwitch', () => {
               'color-contrast': {
                 enabled: theme !== 'dark_dimmed',
               },
-
-              // the 'default' preview does not associate a label with the button
-              'button-name': {
-                enabled: false,
-              },
             },
           })
         })

src/PageLayout/PageLayout.tsx

@@ -482,6 +482,9 @@ Content.displayName = 'PageLayout.Content'
 // PageLayout.Pane
 
 export type PageLayoutPaneProps = {
+  /**
+   * @deprecated position the pane by ordering your markup instead.
+   */
   position?: keyof typeof panePositions | ResponsiveValue<keyof typeof panePositions>
   /**
    * @deprecated Use the `position` prop with a responsive value instead.
@@ -563,11 +566,16 @@ const Pane = React.forwardRef<HTMLDivElement, React.PropsWithChildren<PageLayout
     },
     forwardRef,
   ) => {
-    if (responsivePosition !== undefined) {
-      // eslint-disable-next-line no-console
-      console.warn(
-        'The `position` prop will be removed on the next major version. You should order your markup as you want it to render instead.',
-      )
+    if (__DEV__ && process.env.NODE_ENV !== 'test') {
+      // We don't want these warnings to show up on tests because it fails the tests (at dotcom) due to not extecting a warning.
+      // Practically, this is not a conditional hook, it is just making sure this hook runs only on DEV not PROD.
+      // eslint-disable-next-line react-hooks/rules-of-hooks
+      React.useEffect(() => {
+        warning(
+          responsivePosition !== undefined,
+          'The `position` prop will be removed on the next major version. You should order your markup as you want it to render instead.',
+        )
+      }, [responsivePosition])
     }
 
     // Combine position and positionWhenNarrow for backwards compatibility

src/SplitPageLayout/SplitPageLayout.test.tsx

@@ -17,7 +17,8 @@ describe('SplitPageLayout', () => {
   })
 
   it('renders default layout', () => {
-    const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
+    // Disabling them for now because we are not expecting them in the tests.
+    // const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
     const {container} = render(
       <ThemeProvider>
         <SplitPageLayout>
@@ -28,21 +29,22 @@ describe('SplitPageLayout', () => {
         </SplitPageLayout>
       </ThemeProvider>,
     )
-    expect(spy).toHaveBeenCalled()
+    // expect(spy).toHaveBeenCalled()
 
     expect(container).toMatchSnapshot()
   })
 
   it('renders Pane with a custom ID', () => {
-    const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
+    // Disabling them for now because we are not expecting them in the tests.
+    // const spy = jest.spyOn(console, 'warn').mockImplementation(() => {})
     const {getByText} = render(
       <ThemeProvider>
         <SplitPageLayout>
           <SplitPageLayout.Pane id="customId">Pane Content</SplitPageLayout.Pane>
         </SplitPageLayout>
       </ThemeProvider>,
     )
-    expect(spy).toHaveBeenCalled()
+    // expect(spy).toHaveBeenCalled()
 
     const pane = getByText('Pane Content')
 

src/ToggleSwitch/ToggleSwitch.tsx

@@ -7,6 +7,7 @@ import Text from '../Text'
 import {get} from '../constants'
 import {useProvidedStateOrCreate} from '../hooks'
 import sx, {BetterSystemStyleObject, SxProp} from '../sx'
+import VisuallyHidden from '../_VisuallyHidden'
 import {CellAlignment} from '../DataTable/column'
 
 const TRANSITION_DURATION = '80ms'
@@ -251,8 +252,7 @@ const ToggleSwitch: React.FC<React.PropsWithChildren<ToggleSwitchProps>> = ({
         fontSize={size === 'small' ? 0 : 1}
         mx={2}
         aria-hidden="true"
-        sx={{position: 'relative', cursor: 'pointer'}}
-        onClick={handleToggleClick}
+        sx={{position: 'relative'}}
       >
         <Box textAlign="right" sx={isOn ? null : hiddenTextStyles}>
           On
@@ -265,11 +265,13 @@ const ToggleSwitch: React.FC<React.PropsWithChildren<ToggleSwitchProps>> = ({
         onClick={handleToggleClick}
         aria-labelledby={ariaLabelledby}
         aria-describedby={ariaDescribedby}
-        aria-pressed={isOn}
+        aria-checked={isOn}
+        role="switch"
         checked={isOn}
         size={size}
         disabled={!acceptsInteraction}
       >
+        <VisuallyHidden>{isOn ? 'On' : 'Off'}</VisuallyHidden>
         <Box aria-hidden="true" display="flex" alignItems="center" width="100%" height="100%" overflow="hidden">
           <Box
             flexGrow={1}

src/UnderlineNav2/UnderlineNav.tsx

@@ -1,4 +1,4 @@
-import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject, useEffect} from 'react'
+import React, {useRef, forwardRef, useCallback, useState, MutableRefObject, RefObject} from 'react'
 import Box from '../Box'
 import sx, {merge, BetterSystemStyleObject, SxProp} from '../sx'
 import {UnderlineNavContext} from './UnderlineNavContext'
@@ -175,18 +175,13 @@ export const UnderlineNav = forwardRef(
     // This is the case where the viewport is too narrow to show any list item with the more menu. In this case, we only show the dropdown
     const onlyMenuVisible = responsiveProps.items.length === 0
 
-    if (__DEV__) {
-      // Practically, this is not a conditional hook, it is just making sure this hook runs only on DEV not PROD.
-      // eslint-disable-next-line react-hooks/rules-of-hooks
-      useEffect(() => {
-        // Address illegal state where there are multiple items that have `aria-current='page'` attribute
-        const activeElements = validChildren.filter(child => {
-          return child.props['aria-current'] !== undefined
-        })
-        invariant(activeElements.length <= 1, 'Only one current element is allowed')
-        invariant(ariaLabel, 'Use the `aria-label` prop to provide an accessible label for assistive technology')
-      })
-    }
+    // Address illegal state where there are multiple items that have `aria-current='page'` attribute
+    const activeElements = validChildren.filter(child => {
+      return child.props['aria-current'] !== undefined
+    })
+    invariant(activeElements.length <= 1, 'Only one current element is allowed')
+    // Throw an error if aria-label is not provided
+    invariant(ariaLabel, 'Use the `aria-label` prop to provide an accessible label for assistive technology')
 
     function getItemsWidth(itemText: string): number {
       return noIconChildWidthArray.find(item => item.text === itemText)?.width ?? 0
@@ -310,18 +305,7 @@ export const UnderlineNav = forwardRef(
           ref={navRef}
         >
           <NavigationList sx={ulStyles} ref={listRef} role="list">
-            {listItems.map(listItem => {
-              return (
-                <Box
-                  key={listItem.props.children}
-                  as="li"
-                  sx={{display: 'flex', flexDirection: 'column', alignItems: 'center'}}
-                >
-                  {listItem}
-                </Box>
-              )
-            })}
-
+            {listItems}
             {menuItems.length > 0 && (
               <MoreMenuListItem ref={moreMenuRef}>
                 {!onlyMenuVisible && <Box sx={getDividerStyle(theme)}></Box>}

src/UnderlineNav2/UnderlineNavItem.tsx

@@ -114,43 +114,45 @@ export const UnderlineNavItem = forwardRef(
     )
 
     return (
-      <Link
-        ref={ref}
-        as={Component}
-        href={href}
-        aria-current={ariaCurrent}
-        onKeyDown={keyDownHandler}
-        onClick={clickHandler}
-        sx={merge<BetterSystemStyleObject>(getLinkStyles(theme, ariaCurrent), sxProp as SxProp)}
-        {...props}
-      >
-        {iconsVisible && Icon && (
-          <Box as="span" data-component="icon" sx={iconWrapStyles}>
-            <Icon />
-          </Box>
-        )}
-        {children && (
-          <Box
-            as="span"
-            data-component="text"
-            data-content={children}
-            sx={Boolean(ariaCurrent) && ariaCurrent !== 'false' ? {fontWeight: 600} : {}}
-          >
-            {children}
-          </Box>
-        )}
-        {loadingCounters ? (
-          <Box as="span" data-component="counter" sx={counterStyles}>
-            <LoadingCounter />
-          </Box>
-        ) : (
-          counter !== undefined && (
+      <Box as="li" sx={{display: 'flex', flexDirection: 'column', alignItems: 'center'}}>
+        <Link
+          ref={ref}
+          as={Component}
+          href={href}
+          aria-current={ariaCurrent}
+          onKeyDown={keyDownHandler}
+          onClick={clickHandler}
+          sx={merge<BetterSystemStyleObject>(getLinkStyles(theme, ariaCurrent), sxProp as SxProp)}
+          {...props}
+        >
+          {iconsVisible && Icon && (
+            <Box as="span" data-component="icon" sx={iconWrapStyles}>
+              <Icon />
+            </Box>
+          )}
+          {children && (
+            <Box
+              as="span"
+              data-component="text"
+              data-content={children}
+              sx={Boolean(ariaCurrent) && ariaCurrent !== 'false' ? {fontWeight: 600} : {}}
+            >
+              {children}
+            </Box>
+          )}
+          {loadingCounters ? (
             <Box as="span" data-component="counter" sx={counterStyles}>
-              <CounterLabel>{counter}</CounterLabel>
+              <LoadingCounter />
             </Box>
-          )
-        )}
-      </Link>
+          ) : (
+            counter !== undefined && (
+              <Box as="span" data-component="counter" sx={counterStyles}>
+                <CounterLabel>{counter}</CounterLabel>
+              </Box>
+            )
+          )}
+        </Link>
+      </Box>
     )
   },
 ) as PolymorphicForwardRefComponent<'a', UnderlineNavItemProps>

src/__tests__/ToggleSwitch.test.tsx

@@ -23,7 +23,7 @@ it('renders a switch that is turned off', () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
 })
 it('renders a switch that is turned on', () => {
   const {getByLabelText} = render(
@@ -34,7 +34,7 @@ it('renders a switch that is turned on', () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
 })
 it('renders a switch that is disabled', async () => {
   const user = userEvent.setup()
@@ -46,9 +46,9 @@ it('renders a switch that is disabled', async () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
 })
 it("renders a switch who's state is loading", async () => {
   const user = userEvent.setup()
@@ -62,9 +62,9 @@ it("renders a switch who's state is loading", async () => {
   const loadingSpinner = container.querySelector('svg')
 
   expect(loadingSpinner).toBeDefined()
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
 })
 it('switches from off to on uncontrolled', async () => {
   const user = userEvent.setup()
@@ -76,24 +76,9 @@ it('switches from off to on uncontrolled', async () => {
   )
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
-})
-it('switches from off to on uncontrolled when clicking on status label', async () => {
-  const user = userEvent.setup()
-  const {getByLabelText, getByText} = render(
-    <>
-      <div id="switchLabel">{SWITCH_LABEL_TEXT}</div>
-      <ToggleSwitch aria-labelledby="switchLabel" />
-    </>,
-  )
-  const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
-  const toggleSwitchStatusLabel = getByText('Off')
-
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
-  await user.click(toggleSwitchStatusLabel)
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
 })
 it('switches from off to on with a controlled prop', async () => {
   const user = userEvent.setup()
@@ -114,9 +99,9 @@ it('switches from off to on with a controlled prop', async () => {
   const {getByLabelText} = render(<ControlledSwitchComponent />)
   const toggleSwitch = getByLabelText(SWITCH_LABEL_TEXT)
 
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'false')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'false')
   await user.click(toggleSwitch)
-  expect(toggleSwitch).toHaveAttribute('aria-pressed', 'true')
+  expect(toggleSwitch).toHaveAttribute('aria-checked', 'true')
 })
 it('calls onChange when the switch is toggled', async () => {
   const user = userEvent.setup()