Resolve 5 SonarQube issues across components and tests

[sonarqube-agent]

This PR was automatically created by the Remediation Agent's Scheduled backlog remediation feature.

Why these issues? Issues were selected by severity level, prioritizing blockers and majors that impact code correctness and maintainability. The fixes address test reliability, code readability, and performance concerns that are essential for a component library's quality standards.

This PR fixes critical static analysis violations including two test cases missing assertions, nested ternary operations that reduced code readability, and a React context provider issue causing unnecessary re-renders. These fixes improve code quality, maintainability, and ensure proper test coverage across the codebase.

View Project in SonarCloud

---

Fixed Issues

typescript:S6481 - The object passed as the value prop to the Context provider changes every render. To fix this consider wrapping it in a useMemo hook. • MAJOR • View issue

Location: src/components/unstyled/UnstyledChevronAccordion/UnstyledChevronAccordion.tsx:163

Why is this an issue?

Whenever the value property of React context changes, React will rerender the context and all its child nodes and consumers. In JavaScript, things like object literals or function expressions will create a new identity every time they are evaluated. Such constructions should not be directly used as context value because React will always consider they have changed. This can significantly impact performance.

What changed

Adds the useMemo import from React, which is required by the new ControlledAccordion component that wraps the context provider value in useMemo to prevent unnecessary re-renders caused by creating a new object literal on every render.

--- a/src/components/unstyled/UnstyledChevronAccordion/UnstyledChevronAccordion.tsx
+++ b/src/components/unstyled/UnstyledChevronAccordion/UnstyledChevronAccordion.tsx
@@ -1,1 +1,1 @@
-import React, { ReactNode } from "react";
+import React, { ReactNode, useMemo } from "react";

typescript:S3358 - Extract this nested ternary operation into an independent statement. • MAJOR • View issue

Location: src/components/owa/pupil/lesson/OakLessonNavItem/OakLessonNavItem.tsx:162

Why is this an issue?

Nested ternaries are hard to read and can make the order of operations complex to understand.

What changed

This hunk replaces a nested ternary expression with an if/else-if/else chain. The original code used a ternary within a ternary (isDisabled && disabledBackgroundColor ? disabledBackgroundColor : progress === "not-started" ? notStartedBackgroundColor : backgroundColor), which violated the rule against nested ternary operations. By converting this to an explicit if/else-if/else block, the nested ternary is eliminated, making the code more readable and resolving the static analysis warning about extracting the nested ternary operation into an independent statement.

--- a/src/components/owa/pupil/lesson/OakLessonNavItem/OakLessonNavItem.tsx
+++ b/src/components/owa/pupil/lesson/OakLessonNavItem/OakLessonNavItem.tsx
@@ -159,6 +159,8 @@ export const OakLessonNavItem = <C extends ElementType = "a">(
-  const resolvedBackgroundColor =
-    isDisabled && disabledBackgroundColor
-      ? disabledBackgroundColor
-      : progress === "not-started"
-        ? notStartedBackgroundColor
-        : backgroundColor;
+  let resolvedBackgroundColor;
+  if (isDisabled && disabledBackgroundColor) {
+    resolvedBackgroundColor = disabledBackgroundColor;
+  } else if (progress === "not-started") {
+    resolvedBackgroundColor = notStartedBackgroundColor;
+  } else {
+    resolvedBackgroundColor = backgroundColor;
+  }

typescript:S2699 - Add at least one assertion to this test case. • BLOCKER • View issue

Location: src/components/messaging-and-feedback/OakModalCenter/OakModalCenter.test.tsx:53

Why is this an issue?

An assertion is a statement within the unit test that checks whether a particular condition is true or false. It defines the expected behavior of the unit being tested. Assertions are used to express the test’s expected outcome, and they are the criteria against which the actual output of the unit is evaluated.

What changed

Changes queryAllByTestId to queryByTestId, which is a supporting change for the assertion fix in hunk 2. queryByTestId returns null when no element is found, making it compatible with the .toBeNull() assertion that will be used to actually verify the test's expected outcome.

--- a/src/components/messaging-and-feedback/OakModalCenter/OakModalCenter.test.tsx
+++ b/src/components/messaging-and-feedback/OakModalCenter/OakModalCenter.test.tsx
@@ -56,1 +56,1 @@ describe(OakModalCenter, () => {
-    const { queryAllByTestId } = renderWithTheme(
+    const { queryByTestId } = renderWithTheme(

typescript:S2699 - Add at least one assertion to this test case. • BLOCKER • View issue

Location: src/components/owa/OakCodeRenderer/OakCodeRenderer.test.tsx:19

Why is this an issue?

An assertion is a statement within the unit test that checks whether a particular condition is true or false. It defines the expected behavior of the unit being tested. Assertions are used to express the test’s expected outcome, and they are the criteria against which the actual output of the unit is evaluated.

What changed

This hunk fixes a test case that had no effective assertion. The original code expect(getByText("output")).toBeInTheDocument was merely referencing the toBeInTheDocument property without calling it as a function, meaning no assertion was actually executed. By adding parentheses to make it toBeInTheDocument(), the matcher is now properly invoked, turning it into a real assertion that validates the test's expected outcome.

--- a/src/components/owa/OakCodeRenderer/OakCodeRenderer.test.tsx
+++ b/src/components/owa/OakCodeRenderer/OakCodeRenderer.test.tsx
@@ -23,1 +23,1 @@ describe("OakCodeRenderer", () => {
-    expect(getByText("output")).toBeInTheDocument;
+    expect(getByText("output")).toBeInTheDocument();

typescript:S3358 - Extract this nested ternary operation into an independent statement. • MAJOR • View issue

Location: src/components/internal-components/InternalShadowRoundButton/InternalShadowRoundButton.tsx:170

Why is this an issue?

Nested ternaries are hard to read and can make the order of operations complex to understand.

What changed

This hunk extracts the nested ternary logic into a separate variable iconColorFilter. The original code had a nested ternary: props.disabled ? disabledIconColor : defaultIconColor ? defaultIconColor : null, which violated the rule against nested ternary operations. By computing the value in a standalone statement using disabled ? disabledIconColor : defaultIconColor ?? null, the nested ternary is eliminated. The nullish coalescing operator (??) replaces the inner ternary (defaultIconColor ? defaultIconColor : null) in a cleaner, non-nested way.

--- a/src/components/internal-components/InternalShadowRoundButton/InternalShadowRoundButton.tsx
+++ b/src/components/internal-components/InternalShadowRoundButton/InternalShadowRoundButton.tsx
@@ -161,0 +162,4 @@ export const InternalShadowRoundButton = <C extends ElementType = "button">(
+  const iconColorFilter = disabled
+    ? disabledIconColor
+    : defaultIconColor ?? null;
+

Have a suggestion or found an issue? Share your feedback here.

---

SonarQube Remediation Agent uses AI. Check for mistakes.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
oak-components-storybook	Ready	Preview, Comment	Jun 8, 2026 9:04am