diff --git a/src/components/forms/__tests__/selection-plan-form.test.js b/src/components/forms/__tests__/selection-plan-form.test.js
index 35bce9f58..305384928 100644
--- a/src/components/forms/__tests__/selection-plan-form.test.js
+++ b/src/components/forms/__tests__/selection-plan-form.test.js
@@ -61,6 +61,8 @@ jest.mock("openstack-uicore-foundation/lib/utils/query-actions", () => ({
queryEventTypes: jest.fn()
}));
+jest.mock("../../../hooks/useScrollToError", () => jest.fn());
+
jest.mock("../../mui/formik-inputs/mui-formik-datetimepicker", () => ({
__esModule: true,
default: ({ name }) =>
diff --git a/src/components/forms/selection-plan-form.js b/src/components/forms/selection-plan-form.js
index 416062d9e..4ba555944 100644
--- a/src/components/forms/selection-plan-form.js
+++ b/src/components/forms/selection-plan-form.js
@@ -20,7 +20,7 @@ import { epochToMomentTimeZone } from "openstack-uicore-foundation/lib/utils/met
import Box from "@mui/material/Box";
import Tab from "@mui/material/Tab";
import Tabs from "@mui/material/Tabs";
-import { scrollToError } from "../../utils/methods";
+import useScrollToError from "../../hooks/useScrollToError";
import MainTab from "./selection-plan-form/main-tab";
import TrackGroupsTab from "./selection-plan-form/track-groups-tab";
import EventTypesTab from "./selection-plan-form/event-types-tab";
@@ -111,12 +111,13 @@ const SelectionPlanForm = (props) => {
});
useEffect(() => {
- scrollToError(propsErrors);
formik.setErrors(
propsErrors && Object.keys(propsErrors).length > 0 ? propsErrors : {}
);
}, [propsErrors]);
+ useScrollToError(formik, true, setActiveTab);
+
// Sync sub-resource arrays from Redux without resetting user-editable main tab fields
useEffect(() => {
formik.setValues((current) => ({
diff --git a/src/components/forms/selection-plan-form/email-templates-tab.js b/src/components/forms/selection-plan-form/email-templates-tab.js
index c822f0bbe..62a7e67d3 100644
--- a/src/components/forms/selection-plan-form/email-templates-tab.js
+++ b/src/components/forms/selection-plan-form/email-templates-tab.js
@@ -37,6 +37,7 @@ const EmailTemplatesTab = ({ hidden }) => {
{
{
{
{
control={
@@ -65,6 +67,7 @@ const MainTab = ({ hidden, currentSummit }) => {
control={
@@ -77,6 +80,7 @@ const MainTab = ({ hidden, currentSummit }) => {
control={
@@ -89,6 +93,7 @@ const MainTab = ({ hidden, currentSummit }) => {
control={
@@ -124,6 +129,7 @@ const MainTab = ({ hidden, currentSummit }) => {
diff --git a/src/hooks/__tests__/useScrollToError.test.js b/src/hooks/__tests__/useScrollToError.test.js
new file mode 100644
index 000000000..a47ce5043
--- /dev/null
+++ b/src/hooks/__tests__/useScrollToError.test.js
@@ -0,0 +1,131 @@
+// @testing-library/react 12 (React 16) does not export renderHook; use a
+// lightweight component wrapper instead.
+import "@testing-library/jest-dom";
+import React, { useState } from "react";
+import { act, render, screen } from "@testing-library/react";
+import { useFormik } from "formik";
+import useScrollToError from "../useScrollToError";
+
+window.HTMLElement.prototype.scrollIntoView = jest.fn();
+
+// jsdom implements no layout, so offsetParent is always null regardless of
+// CSS. Reflect the one hiding mechanism this hook cares about (the `hidden`
+// attribute) so tests can simulate real display:none semantics.
+beforeAll(() => {
+ Object.defineProperty(window.HTMLElement.prototype, "offsetParent", {
+ configurable: true,
+ get() {
+ let node = this;
+ while (node) {
+ if (node.hidden) return null;
+ node = node.parentElement;
+ }
+ return document.body;
+ }
+ });
+});
+
+beforeEach(() => {
+ window.HTMLElement.prototype.scrollIntoView.mockClear();
+});
+
+const flushDoubleRaf = () =>
+ act(
+ () =>
+ new Promise((resolve) => {
+ requestAnimationFrame(() => requestAnimationFrame(resolve));
+ })
+ );
+
+const TabbedHarness = ({ onActiveTabChange }) => {
+ const [activeTab, setActiveTab] = useState("b");
+ const formik = useFormik({
+ initialValues: { name: "" },
+ validate: (values) => (values.name ? {} : { name: "required" }),
+ onSubmit: () => {}
+ });
+
+ useScrollToError(formik, true, (value) => {
+ setActiveTab(value);
+ onActiveTabChange?.(value);
+ });
+
+ return (
+
+ );
+};
+
+const VisibleHarness = () => {
+ const formik = useFormik({
+ initialValues: { name: "" },
+ validate: (values) => (values.name ? {} : { name: "required" }),
+ onSubmit: () => {}
+ });
+
+ useScrollToError(formik, true, jest.fn());
+
+ return (
+
+ );
+};
+
+const UntaggedHarness = () => {
+ const formik = useFormik({
+ initialValues: { name: "" },
+ validate: (values) => (values.name ? {} : { name: "required" }),
+ onSubmit: () => {}
+ });
+
+ useScrollToError(formik, true);
+
+ return (
+
+ );
+};
+
+describe("useScrollToError (tab-aware)", () => {
+ it("switches to the owning tab and scrolls when the errored field is hidden", async () => {
+ const onActiveTabChange = jest.fn();
+ render();
+
+ await act(async () => {
+ screen.getByText("save").click();
+ });
+ await flushDoubleRaf();
+
+ expect(onActiveTabChange).toHaveBeenCalledWith("a");
+ expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
+ });
+
+ it("does not switch tabs when the errored field is already visible", async () => {
+ render();
+
+ await act(async () => {
+ screen.getByText("save").click();
+ });
+
+ expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
+ });
+
+ it("behaves as before when setActiveTab is not passed", async () => {
+ render();
+
+ await act(async () => {
+ screen.getByText("save").click();
+ });
+
+ expect(window.HTMLElement.prototype.scrollIntoView).toHaveBeenCalled();
+ });
+});
diff --git a/src/hooks/useScrollToError.js b/src/hooks/useScrollToError.js
index 7579e47a8..d218640bd 100644
--- a/src/hooks/useScrollToError.js
+++ b/src/hooks/useScrollToError.js
@@ -27,7 +27,14 @@ function smoothScrollTo(targetScrollTop, duration) {
requestAnimationFrame(animationStep);
}
-const useScrollToError = (formik, relative = false) => {
+// Waits two animation frames so a DOM mutation applied just before this call
+// (e.g. React removing a tabpanel's `hidden` attribute after setActiveTab)
+// has been through layout before we measure/scroll against it.
+function afterNextLayout(callback) {
+ requestAnimationFrame(() => requestAnimationFrame(callback));
+}
+
+const useScrollToError = (formik, relative = false, setActiveTab) => {
const { errors, isValid, isSubmitting } = formik;
const errorArray = Object.keys(errors);
const errorCount = errorArray.length;
@@ -35,32 +42,73 @@ const useScrollToError = (formik, relative = false) => {
useEffect(() => {
if (isValid || errorCount === 0) return;
- const elementsSorted = errorArray
- .reduce((result, error) => {
- const element = document.querySelector(`[name='${error}']`);
- if (!element) return result;
-
- const rect = element.getBoundingClientRect();
- const absoluteTop = rect.top + window.pageYOffset;
-
- result.push({ element, top: absoluteTop });
- return result;
- }, [])
- .sort((a, b) => a.top - b.top);
-
- if (elementsSorted.length === 0) return;
+ const scrollToFirstVisible = () => {
+ const elementsSorted = errorArray
+ .reduce((result, error) => {
+ const element = document.querySelector(`[name='${error}']`);
+ if (!element) return result;
+
+ const rect = element.getBoundingClientRect();
+ const absoluteTop = rect.top + window.pageYOffset;
+
+ result.push({ element, top: absoluteTop });
+ return result;
+ }, [])
+ .sort((a, b) => a.top - b.top);
+
+ if (elementsSorted.length === 0) return;
+
+ const target = elementsSorted[0];
+
+ const offset = 100; // adjust as needed
+ const duration = 500; // 500ms scroll duration
+ const scrollToY = target.top - offset;
+
+ if (relative) {
+ target?.element.scrollIntoView({
+ behavior: "smooth",
+ block: "center"
+ });
+ } else {
+ smoothScrollTo(scrollToY, duration);
+ }
+ };
+
+ if (typeof setActiveTab !== "function") {
+ scrollToFirstVisible(); // unchanged path for every other call site
+ return;
+ }
- const target = elementsSorted[0];
+ // Tab-aware path: panels in a tabbed form are typically mounted-but-
+ // hidden, so `[name=...]` selectors still match fields on an inactive
+ // tab, but measuring/scrolling a `display:none` element is meaningless.
+ // `offsetParent` is `null` for any element hidden via `display:none`,
+ // including via an ancestor's `hidden` attribute.
+ const matches = errorArray
+ .map((error) => document.querySelector(`[name='${error}']`))
+ .filter(Boolean);
+
+ const allMatchesHidden =
+ matches.length > 0 && matches.every((el) => el.offsetParent === null);
+
+ if (!allMatchesHidden) {
+ scrollToFirstVisible();
+ return;
+ }
- const offset = 100; // adjust as needed
- const duration = 500; // 500ms scroll duration
- const scrollToY = target.top - offset;
+ // Every matched field is hidden: jump to the tab that owns the first
+ // errored field, then defer the scroll until it's actually visible.
+ // Tab panels are identified by the `id="tabpanel-"` convention.
+ const panelId = matches[0].closest("[role=\"tabpanel\"]")?.id;
+ const tabValue = panelId?.match(/^tabpanel-(.+)$/)?.[1];
- if (relative) {
- target?.element.scrollIntoView({ behavior: "smooth", block: "center" });
- } else {
- smoothScrollTo(scrollToY, duration);
+ if (!tabValue) {
+ scrollToFirstVisible(); // not inside a tagged tab panel, fall back
+ return;
}
+
+ setActiveTab(tabValue);
+ afterNextLayout(scrollToFirstVisible);
}, [isSubmitting]);
};
diff --git a/src/pages/events/components/__tests__/event-type-dialog.test.js b/src/pages/events/components/__tests__/event-type-dialog.test.js
index 117a18bd6..2fc1e60bc 100644
--- a/src/pages/events/components/__tests__/event-type-dialog.test.js
+++ b/src/pages/events/components/__tests__/event-type-dialog.test.js
@@ -8,6 +8,7 @@ import {
} from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import EventTypeDialog from "../event-type-dialog";
+import useScrollToError from "../../../../hooks/useScrollToError";
jest.mock("i18n-react/dist/i18n-react", () => ({
translate: jest.fn((key) => key)
@@ -149,6 +150,25 @@ describe("EventTypeDialog", () => {
expect(screen.getByTestId("textfield-name")).toBeInTheDocument();
});
+ it("wires setActiveTab into useScrollToError for tab-aware error scrolling", () => {
+ renderDialog();
+
+ expect(useScrollToError).toHaveBeenCalledWith(
+ expect.anything(),
+ true,
+ expect.any(Function)
+ );
+ });
+
+ it("tags both tabpanels with their owning tab value", () => {
+ renderDialog();
+
+ expect(document.getElementById("tabpanel-main")).toBeInTheDocument();
+ expect(
+ document.getElementById("tabpanel-schedule_settings")
+ ).toBeInTheDocument();
+ });
+
it("disables the class_name select once the entity has an id", () => {
renderDialog({ ...BASE_ENTITY, id: 5, class_name: "EVENT_TYPE" });
diff --git a/src/pages/events/components/event-type-dialog.js b/src/pages/events/components/event-type-dialog.js
index 9faa3427e..ba910678c 100644
--- a/src/pages/events/components/event-type-dialog.js
+++ b/src/pages/events/components/event-type-dialog.js
@@ -158,7 +158,7 @@ const EventTypeDialog = ({
const { values, setFieldValue } = formik;
- useScrollToError(formik, true);
+ useScrollToError(formik, true, setActiveTab);
useEffect(() => {
const errorFields = Object.keys(errors || {});
@@ -248,7 +248,11 @@ const EventTypeDialog = ({
-
+
@@ -577,7 +581,11 @@ const EventTypeDialog = ({
)}
-