Address feedback

mongodb · Jun 3, 2024 · f153eb2 · f153eb2
1 parent 0e94c10
commit f153eb2
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 47 deletions.
diff --git a/.changeset/eight-poets-count.md b/.changeset/eight-poets-count.md
@@ -4,13 +4,21 @@
 
 [LG-4087](https://jira.mongodb.org/browse/LG-4087)
 
-- Removed `@leafygreen-ui/portal` dependency and tabs and their corresponding panels are hydrated with initial render
-- Added `@leafygreen-ui/descendants` dependency to handle relationships between `Tabs` and `TabTitle` descendants and between `Tabs` and `TabPanel` descendants
-- Replaced `Box` with `Polymorphic` which allows tab elements to render as any element or component. It is recommended to continue to use a button or link for accessibility purposes
-- Added `forceRenderAllTabPanels` to `Tabs` component. By default, tab panels conditionally render in the DOM. Setting this prop to `true` will render all, non-disabled tab panels in the DOM and hide the non-selected tabs with CSS
-- Added fixed height to tabs
-- Exported class names to customize styling on containers
+- Removes `@leafygreen-ui/portal` dependency
+  - In previous versions, tabs and their corresponding panels do not render on the server and instead portal content in the respective containers after hydration
+  - Moving forward, tabs and their corresponding panels render normally on the server and render content in the respective containers during initial render cycle
+- Adds `@leafygreen-ui/descendants` dependency
+  - Handles relationships between `Tabs` component and `TabTitle` descendants
+  - Handles relationships between `Tabs` component and `TabPanel` descendants
+- Replaces `@leafygreen-ui/box` with `@leafygreen-ui/polymorphic
+  - Allows tab elements to render as any element or component
+  - Note: it is recommended to continue to use a button or link for accessibility purposes
+- Adds `forceRenderAllTabPanels` to `Tabs` component. By default, tab panels conditionally render in the DOM. Setting this prop to `true` will:
+  - Render all non-disabled tab panels in the DOM
+  - Hide the non-selected tab panels with CSS using `display: none;`. This will also remove the non-selected tab panels from the accessibility tree
+- Adds fixed height to tabs
+- Exports class names to customize styling on containers
   - `tabListElementClassName` can be used to select tab list container
   - `tabPanelsElementClassName` can be used to select tab panels container
-- Added `getAllTabPanelsInDOM` test util
-- Updated `getSelectedPanel` test util. It now checks CSS display property to get the selected panel
+- Adds `getAllTabPanelsInDOM` test util
+- Updates `getSelectedPanel` test util. It now checks CSS display property to get the selected panel
diff --git a/packages/tabs/src/TabPanel/TabPanel.tsx b/packages/tabs/src/TabPanel/TabPanel.tsx
@@ -25,18 +25,20 @@ const TabPanel = ({ children, disabled }: PropsWithChildren<TabPanelProps>) => {
 
   const shouldRender = !disabled && (forceRenderAllTabPanels || selected);
 
-  const tabPanelProps = {
-    className: cx({
-      [hiddenTabPanelStyle]: !selected,
-    }),
-    id,
-    role: 'tabpanel',
-    ['aria-labelledby']: relatedTab?.id,
-  } as const;
-
   return (
     <div ref={ref}>
-      {shouldRender ? <div {...tabPanelProps}>{children}</div> : null}
+      {shouldRender ? (
+        <div
+          aria-labelledby={relatedTab?.id}
+          className={cx({
+            [hiddenTabPanelStyle]: !selected,
+          })}
+          id={id}
+          role="tabpanel"
+        >
+          {children}
+        </div>
+      ) : null}
     </div>
   );
 };

diff --git a/packages/tabs/src/TabTitle/TabTitle.styles.ts b/packages/tabs/src/TabTitle/TabTitle.styles.ts
@@ -9,6 +9,9 @@ import {
   typeScales,
 } from '@leafygreen-ui/tokens';
 
+const TAB_TITLE_BODY_1_HEIGHT = 44;
+const TAB_TITLE_BODY_2_HEIGHT = 52;
+
 interface ListTitleMode {
   base: string;
   hover: string;
@@ -104,12 +107,12 @@ export const listTitleFontSize: Record<BaseFontSize, string> = {
   [BaseFontSize.Body1]: css`
     font-size: ${typeScales.body1.fontSize}px;
     line-height: ${typeScales.body1.lineHeight}px;
-    height: 44px;
+    height: ${TAB_TITLE_BODY_1_HEIGHT}px;
   `,
   [BaseFontSize.Body2]: css`
     font-size: ${typeScales.body2.fontSize}px;
     line-height: ${typeScales.body2.lineHeight}px;
-    height: 52px;
+    height: ${TAB_TITLE_BODY_2_HEIGHT}px;
   `,
 };
 

diff --git a/packages/tabs/src/TabTitle/TabTitle.tsx b/packages/tabs/src/TabTitle/TabTitle.tsx
@@ -53,34 +53,32 @@ const TabTitle = InferredPolymorphic<BaseTabTitleProps, 'button'>(
 
     const nodeText = getNodeTextContent(name);
 
-    const componentProps = {
-      ...rest,
-      className: cx(
-        listTitleFontSize[baseFontSize],
-        listTitleStyles,
-        listTitleModeStyles[theme].base,
-        {
-          [listTitleModeStyles[theme].selected]: !disabled && selected,
-          [listTitleModeStyles[theme].hover]: !disabled && !selected,
-          [listTitleModeStyles[theme].disabled]: disabled,
-        },
-        listTitleModeStyles[theme].focus,
-        className,
-      ),
-      disabled,
-      id,
-      name: nodeText,
-      onClick: handleClick,
-      ref,
-      role: 'tab',
-      tabIndex: selected ? 0 : -1,
-      ['aria-controls']: relatedTabPanel?.id,
-      ['aria-selected']: !disabled && selected,
-      ['data-text']: nodeText,
-    } as const;
-
     return (
-      <Component {...componentProps}>
+      <Component
+        aria-controls={relatedTabPanel?.id}
+        aria-selected={!disabled && selected}
+        className={cx(
+          listTitleFontSize[baseFontSize],
+          listTitleStyles,
+          listTitleModeStyles[theme].base,
+          {
+            [listTitleModeStyles[theme].selected]: !disabled && selected,
+            [listTitleModeStyles[theme].hover]: !disabled && !selected,
+            [listTitleModeStyles[theme].disabled]: disabled,
+          },
+          listTitleModeStyles[theme].focus,
+          className,
+        )}
+        data-text={nodeText}
+        disabled={disabled}
+        id={id}
+        name={nodeText}
+        onClick={handleClick}
+        ref={ref}
+        role="tab"
+        tabIndex={selected ? 0 : -1}
+        {...rest}
+      >
         <div className={listTitleChildrenStyles}>{children}</div>
       </Component>
     );