fix: Fix components not rendering id prop

ariakit · Dec 18, 2019 · 33ecd00 · 33ecd00
1 parent d5e1e8f
commit 33ecd00
Show file tree

Hide file tree

Showing 8 changed files with 112 additions and 20 deletions.
diff --git a/packages/reakit/src/Dialog/__tests__/DialogBackdrop-test.tsx b/packages/reakit/src/Dialog/__tests__/DialogBackdrop-test.tsx
@@ -30,3 +30,20 @@ test("render visible", () => {
     </body>
   `);
 });
+
+test("render with id", () => {
+  const { baseElement } = render(<DialogBackdrop id="test" />);
+  expect(baseElement).toMatchInlineSnapshot(`
+    <body>
+      <div>
+        <div
+          class="hidden"
+          hidden=""
+          id="test"
+          role="presentation"
+          style="display: none;"
+        />
+      </div>
+    </body>
+  `);
+});
diff --git a/packages/reakit/src/Id/Id.tsx b/packages/reakit/src/Id/Id.tsx
@@ -58,7 +58,8 @@ export const unstable_useId = createHook<
   },
 
   useProps(options, htmlProps) {
-    return { id: options.id, ...htmlProps };
+    const id = typeof htmlProps.id === "undefined" ? options.id : htmlProps.id;
+    return { ...htmlProps, id };
   }
 });
 

diff --git a/packages/reakit/src/Id/IdGroup.tsx b/packages/reakit/src/Id/IdGroup.tsx
@@ -46,7 +46,8 @@ export const unstable_useIdGroup = createHook<
   },
 
   useProps(options, htmlProps) {
-    return { id: options.id, ...htmlProps };
+    const id = typeof htmlProps.id === "undefined" ? options.id : htmlProps.id;
+    return { ...htmlProps, id };
   }
 });
 

diff --git a/packages/reakit/src/Rover/Rover.ts b/packages/reakit/src/Rover/Rover.ts
@@ -60,7 +60,7 @@ export const useRover = createHook<RoverOptions, RoverHTMLProps>({
     }
   ) {
     const ref = React.useRef<HTMLElement>(null);
-    const stopId = options.stopId || htmlProps.id || options.id;
+    const stopId = options.stopId || options.id || htmlProps.id;
 
     const trulyDisabled = options.disabled && !options.focusable;
     const noFocused = options.currentId == null;

diff --git a/packages/reakit/src/Tab/Tab.ts b/packages/reakit/src/Tab/Tab.ts
@@ -28,31 +28,26 @@ export const useTab = createHook<TabOptions, TabHTMLProps>({
     options,
     { onClick: htmlOnClick, onFocus: htmlOnFocus, ...htmlProps }
   ) {
-    const selected = options.selectedId === options.stopId;
+    const stopId = options.stopId || options.id || htmlProps.id;
+    const selected = options.selectedId === stopId;
 
     const onClick = React.useCallback(() => {
-      if (!options.disabled && !selected) {
-        options.select(options.stopId);
+      if (stopId && !options.disabled && !selected) {
+        options.select(stopId);
       }
-    }, [options.disabled, selected, options.select, options.stopId]);
+    }, [options.disabled, selected, options.select, stopId]);
 
     const onFocus = React.useCallback(() => {
-      if (!options.disabled && !options.manual && !selected) {
-        options.select(options.stopId);
+      if (stopId && !options.disabled && !options.manual && !selected) {
+        options.select(stopId);
       }
-    }, [
-      options.disabled,
-      options.manual,
-      selected,
-      options.select,
-      options.stopId
-    ]);
+    }, [options.disabled, options.manual, selected, options.select, stopId]);
 
     return {
       role: "tab",
-      id: getTabId(options.stopId, options.baseId),
+      id: getTabId(stopId, options.baseId),
       "aria-selected": selected,
-      "aria-controls": getTabPanelId(options.stopId, options.baseId),
+      "aria-controls": getTabPanelId(stopId, options.baseId),
       onClick: useAllCallbacks(onClick, htmlOnClick),
       onFocus: useAllCallbacks(onFocus, htmlOnFocus),
       ...htmlProps

diff --git a/packages/reakit/src/Tab/__tests__/Tab-test.tsx b/packages/reakit/src/Tab/__tests__/Tab-test.tsx
@@ -82,3 +82,23 @@ test("render active selected", () => {
     </body>
   `);
 });
+
+test("render without state props", () => {
+  // @ts-ignore
+  const { baseElement } = render(<Tab id="test">tab</Tab>);
+  expect(baseElement).toMatchInlineSnapshot(`
+    <body>
+      <div>
+        <button
+          aria-controls="test-panel"
+          aria-selected="false"
+          id="test"
+          role="tab"
+          tabindex="-1"
+        >
+          tab
+        </button>
+      </div>
+    </body>
+  `);
+});
diff --git a/packages/reakit/src/Tab/__tests__/TabPanel-test.tsx b/packages/reakit/src/Tab/__tests__/TabPanel-test.tsx
@@ -72,3 +72,60 @@ test("render selected", () => {
     </body>
   `);
 });
+
+test("render selected", () => {
+  const { baseElement } = render(
+    <TabPanel {...props} selectedId="tab">
+      tabpanel
+    </TabPanel>
+  );
+  expect(baseElement).toMatchInlineSnapshot(`
+    <body>
+      <div>
+        <div
+          aria-labelledby="base-tab"
+          id="base-tab-panel"
+          role="tabpanel"
+          tabindex="0"
+        >
+          tabpanel
+        </div>
+      </div>
+    </body>
+  `);
+});
+
+test("render without state props", () => {
+  // @ts-ignore
+  const { baseElement } = render(<TabPanel>tabpanel</TabPanel>);
+  expect(baseElement).toMatchInlineSnapshot(`
+    <body>
+      <div>
+        <div
+          role="tabpanel"
+          tabindex="0"
+        >
+          tabpanel
+        </div>
+      </div>
+    </body>
+  `);
+});
+
+test("render without state props with id", () => {
+  // @ts-ignore
+  const { baseElement } = render(<TabPanel id="test">tabpanel</TabPanel>);
+  expect(baseElement).toMatchInlineSnapshot(`
+    <body>
+      <div>
+        <div
+          id="test"
+          role="tabpanel"
+          tabindex="0"
+        >
+          tabpanel
+        </div>
+      </div>
+    </body>
+  `);
+});
diff --git a/packages/reakit/src/Tab/__utils.ts b/packages/reakit/src/Tab/__utils.ts
@@ -1,7 +1,8 @@
-export function getTabId(stopId: string, prefix?: string, suffix?: string) {
+export function getTabId(stopId?: string, prefix?: string, suffix?: string) {
+  if (!stopId) return undefined;
   return [prefix, stopId, suffix].filter(Boolean).join("-");
 }
 
-export function getTabPanelId(stopId: string, prefix?: string) {
+export function getTabPanelId(stopId?: string, prefix?: string) {
   return getTabId(stopId, prefix, "panel");
 }