kaxil commented on code in PR #76742:
URL: https://github.com/apache/airflow/pull/76742#discussion_r4514254287
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -40,49 +44,92 @@ const INDICATOR_HEIGHT = "2px";
const trimTrailingSlash = (path: string) => path.replace(/\/$/u, "");
-const NavTabLink = ({
- containerWidth,
- icon,
- label,
- matchPaths,
- search,
- value,
-}: { readonly containerWidth: number } & NavTab) => {
+const TabContent = ({ active, children, ...rest }: { readonly active: boolean
} & CenterProps) => (
+ <Center
+ _focus={{ color: active ? "fg" : "brand.solid" }}
+ _hover={{ color: active ? "fg" : "brand.solid" }}
+ borderBottomColor={active ? "brand.solid" : "transparent"}
+ borderBottomWidth={INDICATOR_HEIGHT}
+ color={active ? "fg" : "fg.muted"}
+ fontSize="md"
+ fontWeight={active ? "bold" : "medium"}
+ height="40px"
+ mb={`-${INDICATOR_HEIGHT}`}
+ px={4}
+ transition="all 0.2s ease"
+ {...rest}
+ >
+ {children}
+ </Center>
+);
+
+export const NavTabs = ({ tabs }: Props) => {
+ const { t: translate } = useTranslation();
+ const containerRef = useRef<HTMLDivElement>(null);
+ const measureRef = useRef<HTMLDivElement>(null);
+ const containerWidth = useContainerWidth(containerRef);
+ const [widths, setWidths] = useState<{ more: number; tabs: Array<number>
}>({ more: 0, tabs: [] });
+
const { pathname } = useLocation();
- const resolved = useResolvedPath({ pathname: value, search });
+ const basePath = useResolvedPath(".").pathname;
// NavLink compares the raw location against its resolved target, and a run
id carries ":" and
// "+". Landing on a link where those arrived percent-encoded leaves the two
spellings of the
// same path looking different, so the tab for the page you are on reads as
inactive.
const current = trimTrailingSlash(decodeURIComponent(pathname));
- const target = trimTrailingSlash(decodeURIComponent(resolved.pathname));
const lastSegment = current.split("/").pop() ?? "";
- const active = current === target || (matchPaths ??
[]).includes(lastSegment);
-
- return (
- <NavLink end title={label} to={{ pathname: value, search }}>
- <Center
- _focus={{ color: active ? "fg" : "brand.solid" }}
- _hover={{ color: active ? "fg" : "brand.solid" }}
- aria-current={active ? "page" : undefined}
- borderBottomColor={active ? "brand.solid" : "transparent"}
- borderBottomWidth={INDICATOR_HEIGHT}
- color={active ? "fg" : "fg.muted"}
- fontSize="md"
- fontWeight={active ? "bold" : "medium"}
- height="40px"
- mb={`-${INDICATOR_HEIGHT}`}
- px={4}
- transition="all 0.2s ease"
- >
- {containerWidth > 600 || !Boolean(icon) ? label : icon}
- </Center>
- </NavLink>
+ const activeIndex = tabs.findIndex(
+ ({ matchPaths, value }) =>
+ current === trimTrailingSlash(decodeURIComponent(resolvePath(value,
basePath).pathname)) ||
+ (matchPaths ?? []).includes(lastSegment),
Review Comment:
Plugin routes are mounted at `plugin/:page/*`, so on a sub-route of a
plugin's React app this exact match gives -1, and that plugin's tab, being
last, is the first one moved into More. Could `plugin/` tabs match on prefix?
(A prefix match for every tab would make the `""` index tab match every page.)
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -40,49 +44,92 @@ const INDICATOR_HEIGHT = "2px";
const trimTrailingSlash = (path: string) => path.replace(/\/$/u, "");
-const NavTabLink = ({
- containerWidth,
- icon,
- label,
- matchPaths,
- search,
- value,
-}: { readonly containerWidth: number } & NavTab) => {
+const TabContent = ({ active, children, ...rest }: { readonly active: boolean
} & CenterProps) => (
+ <Center
+ _focus={{ color: active ? "fg" : "brand.solid" }}
+ _hover={{ color: active ? "fg" : "brand.solid" }}
+ borderBottomColor={active ? "brand.solid" : "transparent"}
+ borderBottomWidth={INDICATOR_HEIGHT}
+ color={active ? "fg" : "fg.muted"}
+ fontSize="md"
+ fontWeight={active ? "bold" : "medium"}
+ height="40px"
+ mb={`-${INDICATOR_HEIGHT}`}
+ px={4}
+ transition="all 0.2s ease"
+ {...rest}
+ >
+ {children}
+ </Center>
+);
+
+export const NavTabs = ({ tabs }: Props) => {
+ const { t: translate } = useTranslation();
+ const containerRef = useRef<HTMLDivElement>(null);
+ const measureRef = useRef<HTMLDivElement>(null);
+ const containerWidth = useContainerWidth(containerRef);
+ const [widths, setWidths] = useState<{ more: number; tabs: Array<number>
}>({ more: 0, tabs: [] });
+
const { pathname } = useLocation();
- const resolved = useResolvedPath({ pathname: value, search });
+ const basePath = useResolvedPath(".").pathname;
// NavLink compares the raw location against its resolved target, and a run
id carries ":" and
// "+". Landing on a link where those arrived percent-encoded leaves the two
spellings of the
// same path looking different, so the tab for the page you are on reads as
inactive.
const current = trimTrailingSlash(decodeURIComponent(pathname));
- const target = trimTrailingSlash(decodeURIComponent(resolved.pathname));
const lastSegment = current.split("/").pop() ?? "";
- const active = current === target || (matchPaths ??
[]).includes(lastSegment);
-
- return (
- <NavLink end title={label} to={{ pathname: value, search }}>
- <Center
- _focus={{ color: active ? "fg" : "brand.solid" }}
- _hover={{ color: active ? "fg" : "brand.solid" }}
- aria-current={active ? "page" : undefined}
- borderBottomColor={active ? "brand.solid" : "transparent"}
- borderBottomWidth={INDICATOR_HEIGHT}
- color={active ? "fg" : "fg.muted"}
- fontSize="md"
- fontWeight={active ? "bold" : "medium"}
- height="40px"
- mb={`-${INDICATOR_HEIGHT}`}
- px={4}
- transition="all 0.2s ease"
- >
- {containerWidth > 600 || !Boolean(icon) ? label : icon}
- </Center>
- </NavLink>
+ const activeIndex = tabs.findIndex(
+ ({ matchPaths, value }) =>
+ current === trimTrailingSlash(decodeURIComponent(resolvePath(value,
basePath).pathname)) ||
+ (matchPaths ?? []).includes(lastSegment),
+ );
+ const showLabels = containerWidth > 600;
+ const getTabContent = ({ icon, label }: NavTab) => (showLabels ||
!Boolean(icon) ? label : icon);
+ const moreLabel = showLabels ? (
+ <HStack gap={1}>
+ {translate("moreTabs")}
+ <FiChevronDown />
+ </HStack>
+ ) : (
+ <FiMoreHorizontal />
);
-};
-export const NavTabs = ({ tabs }: Props) => {
- const containerRef = useRef<HTMLDivElement>(null);
- const containerWidth = useContainerWidth(containerRef);
+ // Measure every tab in a hidden copy, including the ones moved to the menu.
The copy is rendered
+ // bold, as the active tab is, so a tab never grows once it is selected.
+ useLayoutEffect(() => {
+ const measured = [...(measureRef.current?.children ?? [])].map(
+ (child) => (child as HTMLElement).offsetWidth,
+ );
+ const more = measured.pop() ?? 0;
+
+ setWidths((previous) =>
+ previous.more === more && previous.tabs.join(",") === measured.join(",")
+ ? previous
+ : { more, tabs: measured },
+ );
+ }, [tabs, showLabels]);
+
+ // Fill the row in order and move whatever does not fit behind "More". The
active tab always
+ // stays in the row, so the page being viewed is never hidden in the menu.
+ const visibleIndexes = new Set(tabs.map((_, index) => index));
+
+ if (widths.tabs.reduce((total, width) => total + width, 0) > containerWidth)
{
Review Comment:
Until the ResizeObserver reports, `containerWidth` is 0, but the layout
effect has already filled `widths.tabs`, so that render moves every tab except
the active one into More. Would `containerWidth > 0 &&` here keep the row as it
is until the panel has been measured?
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -92,9 +139,54 @@ export const NavTabs = ({ tabs }: Props) => {
mb={2}
ref={containerRef}
>
- {tabs.map((tab) => (
- <NavTabLink containerWidth={containerWidth} key={tab.value} {...tab} />
- ))}
+ {tabs.map((tab, index) =>
+ visibleIndexes.has(index) ? (
+ <NavLink end key={tab.value} title={tab.label} to={{ pathname:
tab.value, search: tab.search }}>
+ <TabContent
+ active={index === activeIndex}
+ aria-current={index === activeIndex ? "page" : undefined}
+ >
+ {getTabContent(tab)}
+ </TabContent>
+ </NavLink>
+ ) : undefined,
+ )}
+ {overflowTabs.length > 0 ? (
+ <Menu.Root positioning={{ placement: "bottom-end" }}>
+ <Menu.Trigger asChild>
+ <TabContent
+ active={false}
+ aria-label={translate("moreTabs")}
+ as="button"
+ cursor="pointer"
+ data-testid="nav-tabs-more"
+ title={translate("moreTabs")}
+ >
+ {moreLabel}
+ </TabContent>
+ </Menu.Trigger>
+ <Menu.Content>
+ {overflowTabs.map((tab) => (
+ <Menu.Item asChild key={tab.value} value={tab.value}>
Review Comment:
The index tabs (Dag and Task Overview, Run and Group Task Instances, Mapped
TI, Asset events) have `value: ""`, and the menu treats an empty `value` as
nothing highlighted, so ArrowDown doesn't move off one of them. It only lands
in the menu in a very narrow panel, but something like
``value={`tab:${tab.value}`}`` avoids it.
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -92,9 +139,54 @@ export const NavTabs = ({ tabs }: Props) => {
mb={2}
ref={containerRef}
>
- {tabs.map((tab) => (
- <NavTabLink containerWidth={containerWidth} key={tab.value} {...tab} />
- ))}
+ {tabs.map((tab, index) =>
+ visibleIndexes.has(index) ? (
+ <NavLink end key={tab.value} title={tab.label} to={{ pathname:
tab.value, search: tab.search }}>
+ <TabContent
+ active={index === activeIndex}
+ aria-current={index === activeIndex ? "page" : undefined}
+ >
+ {getTabContent(tab)}
+ </TabContent>
+ </NavLink>
+ ) : undefined,
+ )}
+ {overflowTabs.length > 0 ? (
+ <Menu.Root positioning={{ placement: "bottom-end" }}>
+ <Menu.Trigger asChild>
+ <TabContent
+ active={false}
+ aria-label={translate("moreTabs")}
+ as="button"
+ cursor="pointer"
+ data-testid="nav-tabs-more"
+ title={translate("moreTabs")}
+ >
+ {moreLabel}
+ </TabContent>
+ </Menu.Trigger>
+ <Menu.Content>
+ {overflowTabs.map((tab) => (
+ <Menu.Item asChild key={tab.value} value={tab.value}>
+ <NavLink to={{ pathname: tab.value, search: tab.search }}>
+ {tab.icon}
Review Comment:
For a plugin with an icon image, the `<img alt={view.name}>` next to the
label gives the menu item the accessible name "MyView MyView". Wrapping the
icon in an `aria-hidden` span would avoid that.
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -40,49 +44,92 @@ const INDICATOR_HEIGHT = "2px";
const trimTrailingSlash = (path: string) => path.replace(/\/$/u, "");
-const NavTabLink = ({
- containerWidth,
- icon,
- label,
- matchPaths,
- search,
- value,
-}: { readonly containerWidth: number } & NavTab) => {
+const TabContent = ({ active, children, ...rest }: { readonly active: boolean
} & CenterProps) => (
+ <Center
+ _focus={{ color: active ? "fg" : "brand.solid" }}
+ _hover={{ color: active ? "fg" : "brand.solid" }}
+ borderBottomColor={active ? "brand.solid" : "transparent"}
+ borderBottomWidth={INDICATOR_HEIGHT}
+ color={active ? "fg" : "fg.muted"}
+ fontSize="md"
+ fontWeight={active ? "bold" : "medium"}
+ height="40px"
+ mb={`-${INDICATOR_HEIGHT}`}
+ px={4}
+ transition="all 0.2s ease"
+ {...rest}
+ >
+ {children}
+ </Center>
+);
+
+export const NavTabs = ({ tabs }: Props) => {
+ const { t: translate } = useTranslation();
+ const containerRef = useRef<HTMLDivElement>(null);
+ const measureRef = useRef<HTMLDivElement>(null);
+ const containerWidth = useContainerWidth(containerRef);
+ const [widths, setWidths] = useState<{ more: number; tabs: Array<number>
}>({ more: 0, tabs: [] });
+
const { pathname } = useLocation();
- const resolved = useResolvedPath({ pathname: value, search });
+ const basePath = useResolvedPath(".").pathname;
// NavLink compares the raw location against its resolved target, and a run
id carries ":" and
// "+". Landing on a link where those arrived percent-encoded leaves the two
spellings of the
// same path looking different, so the tab for the page you are on reads as
inactive.
const current = trimTrailingSlash(decodeURIComponent(pathname));
- const target = trimTrailingSlash(decodeURIComponent(resolved.pathname));
const lastSegment = current.split("/").pop() ?? "";
- const active = current === target || (matchPaths ??
[]).includes(lastSegment);
-
- return (
- <NavLink end title={label} to={{ pathname: value, search }}>
- <Center
- _focus={{ color: active ? "fg" : "brand.solid" }}
- _hover={{ color: active ? "fg" : "brand.solid" }}
- aria-current={active ? "page" : undefined}
- borderBottomColor={active ? "brand.solid" : "transparent"}
- borderBottomWidth={INDICATOR_HEIGHT}
- color={active ? "fg" : "fg.muted"}
- fontSize="md"
- fontWeight={active ? "bold" : "medium"}
- height="40px"
- mb={`-${INDICATOR_HEIGHT}`}
- px={4}
- transition="all 0.2s ease"
- >
- {containerWidth > 600 || !Boolean(icon) ? label : icon}
- </Center>
- </NavLink>
+ const activeIndex = tabs.findIndex(
+ ({ matchPaths, value }) =>
+ current === trimTrailingSlash(decodeURIComponent(resolvePath(value,
basePath).pathname)) ||
+ (matchPaths ?? []).includes(lastSegment),
+ );
+ const showLabels = containerWidth > 600;
+ const getTabContent = ({ icon, label }: NavTab) => (showLabels ||
!Boolean(icon) ? label : icon);
+ const moreLabel = showLabels ? (
+ <HStack gap={1}>
Review Comment:
This puts a `<div>` inside the trigger's `<button>`. `<HStack as="span">`
keeps it valid HTML.
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.test.tsx:
##########
@@ -0,0 +1,142 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChakraProvider, defaultSystem } from "@chakra-ui/react";
+import "@testing-library/jest-dom/vitest";
+import { fireEvent, render, screen, within } from "@testing-library/react";
+import { MemoryRouter, Outlet, Route, Routes, useLocation } from
"react-router-dom";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useContainerWidth } from "src/utils";
+
+import { NavTabs, type NavTab } from "./NavTabs";
+
+vi.mock("src/utils", async () => {
+ const actual = await vi.importActual("src/utils");
+
+ return { ...actual, useContainerWidth: vi.fn() };
+});
+
+// Every tab, and the "More" trigger, measures 100px.
+const TAB_WIDTH = 100;
+
+const TABS: Array<NavTab> = Array.from({ length: 9 }, (_, index) => ({
+ label: `Tab ${index}`,
+ value: `tab${index}`,
+}));
+
+const CurrentPath = () => <div
data-testid="current-path">{useLocation().pathname}</div>;
+
+const renderTabs = (initialPath: string, tabs: Array<NavTab> = TABS) =>
+ render(
+ <ChakraProvider value={defaultSystem}>
+ <MemoryRouter initialEntries={[initialPath]}>
+ <Routes>
+ <Route
+ element={
+ <>
+ <NavTabs tabs={tabs} />
+ <Outlet />
+ </>
+ }
+ path="/dag"
+ >
+ <Route element={<CurrentPath />} path=":tab" />
+ </Route>
+ </Routes>
+ </MemoryRouter>
+ </ChakraProvider>,
+ );
+
+const getRowLabels = () =>
+ screen
+ .getAllByRole("link")
+ .filter((link) => link.closest("[role=menu]") === null)
+ .map((link) => link.textContent);
+
+describe("NavTabs", () => {
+ beforeEach(() => {
+ vi.spyOn(HTMLElement.prototype, "offsetWidth",
"get").mockReturnValue(TAB_WIDTH);
+ });
+
+ afterEach(() => {
+ vi.restoreAllMocks();
+ });
+
+ it("renders every tab in the row when they all fit", () => {
+ vi.mocked(useContainerWidth).mockReturnValue(TABS.length * TAB_WIDTH);
+
+ renderTabs("/dag/tab0");
+
+ expect(getRowLabels()).toEqual(TABS.map(({ label }) => label));
+ expect(screen.queryByTestId("nav-tabs-more")).not.toBeInTheDocument();
+ });
+
+ it("moves the tabs that do not fit to the More menu", async () => {
+ vi.mocked(useContainerWidth).mockReturnValue(700);
+
+ renderTabs("/dag/tab0");
+
+ // 700px leaves room for the "More" trigger and six tabs.
+ expect(getRowLabels()).toEqual(["Tab 0", "Tab 1", "Tab 2", "Tab 3", "Tab
4", "Tab 5"]);
+
+ fireEvent.click(screen.getByTestId("nav-tabs-more"));
+ const menu = await screen.findByRole("menu");
+
+ expect(
+ within(menu)
+ .getAllByRole("menuitem")
+ .map((item) => item.textContent),
+ ).toEqual(["Tab 6", "Tab 7", "Tab 8"]);
+
+ fireEvent.click(within(menu).getByText("Tab 7"));
+
+ expect(screen.getByTestId("current-path")).toHaveTextContent("/dag/tab7");
+ });
+
+ it("keeps the active tab in the row when it would overflow", () => {
Review Comment:
Dropping the `matchPaths` clause from `activeIndex`, or the `search` on the
menu `NavLink`, still passes these tests. Could one case pin a tab that is
active through `matchPaths` (like the TI Logs tab), and another check that a
menu item keeps its `search`?
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.test.tsx:
##########
@@ -0,0 +1,142 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChakraProvider, defaultSystem } from "@chakra-ui/react";
+import "@testing-library/jest-dom/vitest";
+import { fireEvent, render, screen, within } from "@testing-library/react";
+import { MemoryRouter, Outlet, Route, Routes, useLocation } from
"react-router-dom";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useContainerWidth } from "src/utils";
+
+import { NavTabs, type NavTab } from "./NavTabs";
+
+vi.mock("src/utils", async () => {
+ const actual = await vi.importActual("src/utils");
+
+ return { ...actual, useContainerWidth: vi.fn() };
+});
+
+// Every tab, and the "More" trigger, measures 100px.
+const TAB_WIDTH = 100;
+
+const TABS: Array<NavTab> = Array.from({ length: 9 }, (_, index) => ({
+ label: `Tab ${index}`,
+ value: `tab${index}`,
+}));
+
+const CurrentPath = () => <div
data-testid="current-path">{useLocation().pathname}</div>;
+
+const renderTabs = (initialPath: string, tabs: Array<NavTab> = TABS) =>
+ render(
+ <ChakraProvider value={defaultSystem}>
+ <MemoryRouter initialEntries={[initialPath]}>
+ <Routes>
+ <Route
+ element={
+ <>
+ <NavTabs tabs={tabs} />
+ <Outlet />
+ </>
+ }
+ path="/dag"
+ >
+ <Route element={<CurrentPath />} path=":tab" />
+ </Route>
+ </Routes>
+ </MemoryRouter>
+ </ChakraProvider>,
+ );
+
+const getRowLabels = () =>
+ screen
+ .getAllByRole("link")
+ .filter((link) => link.closest("[role=menu]") === null)
+ .map((link) => link.textContent);
+
+describe("NavTabs", () => {
+ beforeEach(() => {
+ vi.spyOn(HTMLElement.prototype, "offsetWidth",
"get").mockReturnValue(TAB_WIDTH);
Review Comment:
With every element at 100px the `isFull` stop never changes the result, so
removing it still passes all four tests. A case with uneven widths, where a
wide tab overflows before narrower ones after it, would cover the in-order fill.
##########
airflow-core/src/airflow/ui/src/layouts/Details/NavTabs.tsx:
##########
@@ -40,49 +44,92 @@ const INDICATOR_HEIGHT = "2px";
const trimTrailingSlash = (path: string) => path.replace(/\/$/u, "");
-const NavTabLink = ({
- containerWidth,
- icon,
- label,
- matchPaths,
- search,
- value,
-}: { readonly containerWidth: number } & NavTab) => {
+const TabContent = ({ active, children, ...rest }: { readonly active: boolean
} & CenterProps) => (
+ <Center
+ _focus={{ color: active ? "fg" : "brand.solid" }}
+ _hover={{ color: active ? "fg" : "brand.solid" }}
+ borderBottomColor={active ? "brand.solid" : "transparent"}
+ borderBottomWidth={INDICATOR_HEIGHT}
+ color={active ? "fg" : "fg.muted"}
+ fontSize="md"
+ fontWeight={active ? "bold" : "medium"}
+ height="40px"
+ mb={`-${INDICATOR_HEIGHT}`}
+ px={4}
+ transition="all 0.2s ease"
+ {...rest}
+ >
+ {children}
+ </Center>
+);
+
+export const NavTabs = ({ tabs }: Props) => {
+ const { t: translate } = useTranslation();
+ const containerRef = useRef<HTMLDivElement>(null);
+ const measureRef = useRef<HTMLDivElement>(null);
+ const containerWidth = useContainerWidth(containerRef);
+ const [widths, setWidths] = useState<{ more: number; tabs: Array<number>
}>({ more: 0, tabs: [] });
+
const { pathname } = useLocation();
- const resolved = useResolvedPath({ pathname: value, search });
+ const basePath = useResolvedPath(".").pathname;
// NavLink compares the raw location against its resolved target, and a run
id carries ":" and
// "+". Landing on a link where those arrived percent-encoded leaves the two
spellings of the
// same path looking different, so the tab for the page you are on reads as
inactive.
const current = trimTrailingSlash(decodeURIComponent(pathname));
- const target = trimTrailingSlash(decodeURIComponent(resolved.pathname));
const lastSegment = current.split("/").pop() ?? "";
- const active = current === target || (matchPaths ??
[]).includes(lastSegment);
-
- return (
- <NavLink end title={label} to={{ pathname: value, search }}>
- <Center
- _focus={{ color: active ? "fg" : "brand.solid" }}
- _hover={{ color: active ? "fg" : "brand.solid" }}
- aria-current={active ? "page" : undefined}
- borderBottomColor={active ? "brand.solid" : "transparent"}
- borderBottomWidth={INDICATOR_HEIGHT}
- color={active ? "fg" : "fg.muted"}
- fontSize="md"
- fontWeight={active ? "bold" : "medium"}
- height="40px"
- mb={`-${INDICATOR_HEIGHT}`}
- px={4}
- transition="all 0.2s ease"
- >
- {containerWidth > 600 || !Boolean(icon) ? label : icon}
- </Center>
- </NavLink>
+ const activeIndex = tabs.findIndex(
+ ({ matchPaths, value }) =>
+ current === trimTrailingSlash(decodeURIComponent(resolvePath(value,
basePath).pathname)) ||
+ (matchPaths ?? []).includes(lastSegment),
+ );
+ const showLabels = containerWidth > 600;
+ const getTabContent = ({ icon, label }: NavTab) => (showLabels ||
!Boolean(icon) ? label : icon);
+ const moreLabel = showLabels ? (
+ <HStack gap={1}>
+ {translate("moreTabs")}
+ <FiChevronDown />
+ </HStack>
+ ) : (
+ <FiMoreHorizontal />
);
-};
-export const NavTabs = ({ tabs }: Props) => {
- const containerRef = useRef<HTMLDivElement>(null);
- const containerWidth = useContainerWidth(containerRef);
+ // Measure every tab in a hidden copy, including the ones moved to the menu.
The copy is rendered
+ // bold, as the active tab is, so a tab never grows once it is selected.
+ useLayoutEffect(() => {
+ const measured = [...(measureRef.current?.children ?? [])].map(
+ (child) => (child as HTMLElement).offsetWidth,
Review Comment:
`child.getBoundingClientRect().width` would drop the cast, and it gives
fractional widths like the ResizeObserver value they're compared against.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
