From c21a76485447b7f4ed3c1639f7d05c164d1add94 Mon Sep 17 00:00:00 2001 From: Jackson Hoffart Date: Wed, 5 Nov 2025 13:14:22 +0000 Subject: [PATCH 1/3] fix: update Badge component type safety and props --- src/components/Badge.test.tsx | 105 ++++++++-------------------------- src/components/Badge.tsx | 25 ++++---- 2 files changed, 39 insertions(+), 91 deletions(-) diff --git a/src/components/Badge.test.tsx b/src/components/Badge.test.tsx index 00fd6c7c..48ea0996 100644 --- a/src/components/Badge.test.tsx +++ b/src/components/Badge.test.tsx @@ -23,22 +23,14 @@ describe("Badge component", () => { expect(badge).toHaveClass("border-rmigray-200"); }); - // Type assertions: children must be scalar (string|number) it("disallows ReactNode children for Badge at compile-time", () => { // @ts-expect-error - Badge children must be string | number - render( - - Not allowed - , - ); + render(
Not allowed
); }); it("applies pathwayType styling when variant is 'pathwayType'", () => { const { container } = render( - , + Pathway Type ); const badge = container.firstChild as HTMLElement; @@ -49,10 +41,7 @@ describe("Badge component", () => { it("applies temperature styling when variant is 'temperature'", () => { const { container } = render( - , + 1.5°C ); const badge = container.firstChild as HTMLElement; @@ -63,10 +52,7 @@ describe("Badge component", () => { it("applies year styling when variant is 'year'", () => { const { container } = render( - , + 2050 ); const badge = container.firstChild as HTMLElement; @@ -77,10 +63,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyGlobal'", () => { const { container } = render( - , + Global ); const badge = container.firstChild as HTMLElement; @@ -91,10 +74,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyRegion'", () => { const { container } = render( - , + RegionName ); const badge = container.firstChild as HTMLElement; @@ -105,10 +85,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyCountry'", () => { const { container } = render( - , + CountryName ); const badge = container.firstChild as HTMLElement; @@ -119,10 +96,7 @@ describe("Badge component", () => { it("applies sector styling when variant is 'sector'", () => { const { container } = render( - , + Energy ); const badge = container.firstChild as HTMLElement; @@ -140,16 +114,11 @@ describe("Badge component", () => { }); it("always includes base badge styling", () => { - // Testing that common styles are applied to all variants const { container } = render( - , + Test ); const badge = container.firstChild as HTMLElement; - // Check for common styling classes that should be on all badges expect(badge).toHaveClass("inline-flex"); expect(badge).toHaveClass("items-center"); expect(badge).toHaveClass("rounded-full"); @@ -161,18 +130,13 @@ describe("Badge component", () => { }); it("renders as a span element", () => { - const { container } = render(); + const { container } = render(Test); expect(container.firstChild?.nodeName).toBe("SPAN"); }); - // Tooltip tests it("does not render tooltip when no tooltip is provided", () => { render(No Tooltip); - - // Find the badge span const badge = screen.getByText("No Tooltip"); - - // Check that it's a plain span without tabindex expect(badge).toBeInTheDocument(); expect(badge).not.toHaveAttribute("tabindex"); }); @@ -180,25 +144,16 @@ describe("Badge component", () => { it("uses TextWithTooltip when tooltip is provided", () => { render(With Tooltip); - // Badge text should still be present const badgeText = screen.getByText("With Tooltip"); expect(badgeText).toBeInTheDocument(); - // The outer span from TextWithTooltip should have tabindex attribute - // We need to look for a parent element with tabindex since the badge text itself - // is wrapped in its own span const triggerElement = badgeText.closest("span")?.parentElement; expect(triggerElement).toHaveAttribute("tabindex", "0"); - - // The aria-describedby attribute is added when tooltip is visible expect(triggerElement).not.toHaveAttribute("aria-describedby"); }); - // Testing tooltip visibility requires checking document.body, since tooltips are now in portals it("doesn't show tooltip initially", () => { render(Hover me); - - // Initially the tooltip shouldn't be in document.body const tooltipElement = document.querySelector("[role='tooltip']"); expect(tooltipElement).not.toBeInTheDocument(); }); @@ -211,14 +166,14 @@ describe("Badge component", () => { variant="pathwayType" > Normative - , + ); const badge = screen.getByText("Normative"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0", + "0" ); }); @@ -229,14 +184,14 @@ describe("Badge component", () => { variant="pathwayType" > Direct Policy - , + ); const badge = screen.getByText("Direct Policy"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0", + "0" ); }); @@ -247,14 +202,14 @@ describe("Badge component", () => { variant="sector" > Power - , + ); const badge = screen.getByText("Power"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0", + "0" ); }); @@ -265,14 +220,14 @@ describe("Badge component", () => { variant="sector" > Transport - , + ); const badge = screen.getByText("Transport"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0", + "0" ); }); }); @@ -280,9 +235,9 @@ describe("Badge component", () => { describe("BadgeMaybeAbsent", () => { it("renders 'None' for undefined/null", () => { - const { rerender } = render(); + const { rerender } = render({undefined}); expect(screen.getByText("None")).toBeInTheDocument(); - rerender(); + rerender({null}); expect(screen.getByText("None")).toBeInTheDocument(); }); @@ -301,33 +256,23 @@ describe("BadgeMaybeAbsent", () => { it("uses toLabel only for present values", () => { const toLabel = (v: number) => `Y${v}`; const { rerender } = render( - toLabel={toLabel}>2030, + toLabel={toLabel}>{2030} ); expect(screen.getByText("Y2030")).toBeInTheDocument(); rerender( - - text={undefined} - toLabel={toLabel} - />, + toLabel={toLabel}>{undefined} ); expect(screen.getByText("None")).toBeInTheDocument(); }); it("disallows ReactNode children for BadgeMaybeAbsent at compile-time", () => { // @ts-expect-error - BadgeMaybeAbsent children must be string | number | null | undefined - render( - - Not allowed - , - ); + render(
Not allowed
); }); it("supports noneLabel override", () => { render( - , + {undefined} ); expect(screen.getByText("No Value")).toBeInTheDocument(); }); @@ -335,7 +280,7 @@ describe("BadgeMaybeAbsent", () => { it("renderLabel decorates only string labels", () => { const renderLabel = (label: string) => `**${label}**`; render( - EUROPE, + EUROPE ); expect(screen.getByText("**EUROPE**")).toBeInTheDocument(); }); diff --git a/src/components/Badge.tsx b/src/components/Badge.tsx index 9c312244..bddc2d79 100644 --- a/src/components/Badge.tsx +++ b/src/components/Badge.tsx @@ -6,14 +6,16 @@ interface BadgeProps { children: string | number; tooltip?: string; variant?: - | "default" - | "pathwayType" - | "temperature" - | "year" - | "geographyGlobal" - | "geographyRegion" - | "geographyCountry" - | "sector"; + | "default" + | "pathwayType" + | "temperature" + | "year" + | "geographyGlobal" + | "geographyRegion" + | "geographyCountry" + | "sector" + | "metric" + | "keyFeature"; className?: string; } @@ -104,11 +106,12 @@ export function BadgeMaybeAbsent({ } else if (typeof children === "string" || typeof children === "number") { base = String(children); } else { - // Should be unreachable at runtime due to typing, but keep a safe fallback base = String(children as unknown as string | number); } - const content = - renderLabel && typeof base === "string" ? renderLabel(base, absent) : base; + // Convert ReactNode to string + const content = renderLabel && typeof base === "string" + ? String(renderLabel(base, absent)) + : base; return {content}; } From a89b1f4f7c0580d0bd8ed153cea267554ca81937 Mon Sep 17 00:00:00 2001 From: Jackson Hoffart Date: Wed, 5 Nov 2025 13:15:40 +0000 Subject: [PATCH 2/3] prettier --write . --- src/components/Badge.test.tsx | 66 +++++++++++++++++++---------------- src/components/Badge.tsx | 27 +++++++------- 2 files changed, 49 insertions(+), 44 deletions(-) diff --git a/src/components/Badge.test.tsx b/src/components/Badge.test.tsx index 48ea0996..44d7415a 100644 --- a/src/components/Badge.test.tsx +++ b/src/components/Badge.test.tsx @@ -25,12 +25,16 @@ describe("Badge component", () => { it("disallows ReactNode children for Badge at compile-time", () => { // @ts-expect-error - Badge children must be string | number - render(
Not allowed
); + render( + +
Not allowed
+
, + ); }); it("applies pathwayType styling when variant is 'pathwayType'", () => { const { container } = render( - Pathway Type + Pathway Type, ); const badge = container.firstChild as HTMLElement; @@ -40,9 +44,7 @@ describe("Badge component", () => { }); it("applies temperature styling when variant is 'temperature'", () => { - const { container } = render( - 1.5°C - ); + const { container } = render(1.5°C); const badge = container.firstChild as HTMLElement; expect(badge).toHaveClass("bg-rmired-100"); @@ -51,9 +53,7 @@ describe("Badge component", () => { }); it("applies year styling when variant is 'year'", () => { - const { container } = render( - 2050 - ); + const { container } = render(2050); const badge = container.firstChild as HTMLElement; expect(badge).toHaveClass("bg-rmiblue-100"); @@ -63,7 +63,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyGlobal'", () => { const { container } = render( - Global + Global, ); const badge = container.firstChild as HTMLElement; @@ -74,7 +74,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyRegion'", () => { const { container } = render( - RegionName + RegionName, ); const badge = container.firstChild as HTMLElement; @@ -85,7 +85,7 @@ describe("Badge component", () => { it("applies geography styling when variant is 'geographyCountry'", () => { const { container } = render( - CountryName + CountryName, ); const badge = container.firstChild as HTMLElement; @@ -95,9 +95,7 @@ describe("Badge component", () => { }); it("applies sector styling when variant is 'sector'", () => { - const { container } = render( - Energy - ); + const { container } = render(Energy); const badge = container.firstChild as HTMLElement; expect(badge).toHaveClass("bg-solar-100"); @@ -114,9 +112,7 @@ describe("Badge component", () => { }); it("always includes base badge styling", () => { - const { container } = render( - Test - ); + const { container } = render(Test); const badge = container.firstChild as HTMLElement; expect(badge).toHaveClass("inline-flex"); @@ -166,14 +162,14 @@ describe("Badge component", () => { variant="pathwayType" > Normative -
+ , ); const badge = screen.getByText("Normative"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0" + "0", ); }); @@ -184,14 +180,14 @@ describe("Badge component", () => { variant="pathwayType" > Direct Policy - + , ); const badge = screen.getByText("Direct Policy"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0" + "0", ); }); @@ -202,14 +198,14 @@ describe("Badge component", () => { variant="sector" > Power - + , ); const badge = screen.getByText("Power"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0" + "0", ); }); @@ -220,14 +216,14 @@ describe("Badge component", () => { variant="sector" > Transport - + , ); const badge = screen.getByText("Transport"); expect(badge).toBeInTheDocument(); expect(badge.closest("span")?.parentElement).toHaveAttribute( "tabindex", - "0" + "0", ); }); }); @@ -235,7 +231,9 @@ describe("Badge component", () => { describe("BadgeMaybeAbsent", () => { it("renders 'None' for undefined/null", () => { - const { rerender } = render({undefined}); + const { rerender } = render( + {undefined}, + ); expect(screen.getByText("None")).toBeInTheDocument(); rerender({null}); expect(screen.getByText("None")).toBeInTheDocument(); @@ -256,23 +254,29 @@ describe("BadgeMaybeAbsent", () => { it("uses toLabel only for present values", () => { const toLabel = (v: number) => `Y${v}`; const { rerender } = render( - toLabel={toLabel}>{2030} + toLabel={toLabel}>{2030}, ); expect(screen.getByText("Y2030")).toBeInTheDocument(); rerender( - toLabel={toLabel}>{undefined} + toLabel={toLabel}> + {undefined} + , ); expect(screen.getByText("None")).toBeInTheDocument(); }); it("disallows ReactNode children for BadgeMaybeAbsent at compile-time", () => { // @ts-expect-error - BadgeMaybeAbsent children must be string | number | null | undefined - render(
Not allowed
); + render( + +
Not allowed
+
, + ); }); it("supports noneLabel override", () => { render( - {undefined} + {undefined}, ); expect(screen.getByText("No Value")).toBeInTheDocument(); }); @@ -280,7 +284,7 @@ describe("BadgeMaybeAbsent", () => { it("renderLabel decorates only string labels", () => { const renderLabel = (label: string) => `**${label}**`; render( - EUROPE + EUROPE, ); expect(screen.getByText("**EUROPE**")).toBeInTheDocument(); }); diff --git a/src/components/Badge.tsx b/src/components/Badge.tsx index bddc2d79..2113fb76 100644 --- a/src/components/Badge.tsx +++ b/src/components/Badge.tsx @@ -6,16 +6,16 @@ interface BadgeProps { children: string | number; tooltip?: string; variant?: - | "default" - | "pathwayType" - | "temperature" - | "year" - | "geographyGlobal" - | "geographyRegion" - | "geographyCountry" - | "sector" - | "metric" - | "keyFeature"; + | "default" + | "pathwayType" + | "temperature" + | "year" + | "geographyGlobal" + | "geographyRegion" + | "geographyCountry" + | "sector" + | "metric" + | "keyFeature"; className?: string; } @@ -110,8 +110,9 @@ export function BadgeMaybeAbsent({ } // Convert ReactNode to string - const content = renderLabel && typeof base === "string" - ? String(renderLabel(base, absent)) - : base; + const content = + renderLabel && typeof base === "string" + ? String(renderLabel(base, absent)) + : base; return {content}; } From e474951404199d51b7357910185e9c09936c2f76 Mon Sep 17 00:00:00 2001 From: Jackson Hoffart Date: Wed, 5 Nov 2025 13:22:29 +0000 Subject: [PATCH 3/3] fix: renderLabel returns string directly --- src/components/Badge.tsx | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/components/Badge.tsx b/src/components/Badge.tsx index 2113fb76..4dc930b4 100644 --- a/src/components/Badge.tsx +++ b/src/components/Badge.tsx @@ -55,13 +55,10 @@ const Badge: React.FC = ({ ? `${badgeStylesBase} ${className}` : badgeStylesBase; - // If no tooltip, just return the basic badge - // If no tooltip, just return the basic badge if (!tooltip) { return {children}; } - // With tooltip, use the TextWithTooltip component return ( {children}} @@ -72,7 +69,6 @@ const Badge: React.FC = ({ }; export default Badge; -// --- value-aware helpers (schema-agnostic) -------------------- export type BadgeMaybeAbsentProps = Omit< React.ComponentProps, @@ -85,7 +81,7 @@ export type BadgeMaybeAbsentProps = Omit< /** Visible text for the "None" case (default "None"). */ noneLabel?: string; /** Optional decorator for the final label (e.g., highlight search matches). */ - renderLabel?: (label: string, isAbsent: boolean) => React.ReactNode; + renderLabel?: (label: string, isAbsent: boolean) => string | number; }; export function BadgeMaybeAbsent({ @@ -109,10 +105,8 @@ export function BadgeMaybeAbsent({ base = String(children as unknown as string | number); } - // Convert ReactNode to string const content = - renderLabel && typeof base === "string" - ? String(renderLabel(base, absent)) - : base; - return {content}; + renderLabel && typeof base === "string" ? renderLabel(base, absent) : base; + + return {String(content)}; }