From 342533e1286b7d045f28a8047846210a6d86fb4b Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Fri, 4 Jul 2025 17:28:29 +0000 Subject: [PATCH] refactor(CustomFields): Implement UI-only reordering with Save button This commit refactors the custom field ordering functionality to support UI-only reordering with an explicit "Save Order" button. This replaces the previous approach where each move operation updated the backend. Changes include: - **Frontend State Management:** - `CustomFields.tsx` now manages a local state (`displayedFields`) for the order of custom fields. - Reordering operations ("Move Up"/"Move Down") update this local state directly without immediate backend calls. - **"Save Order" Button:** - A "Save Order" button has been added to the UI. - This button is enabled only when changes to the order have been made locally. - Clicking "Save Order" triggers API calls to persist the new order of all changed fields to the backend. - **API Calls:** - `editCustomField` is now called in a batch when "Save Order" is pressed, only for fields whose order has effectively changed. - **Local Order Integrity:** - The `onMove` handler now ensures that the local `order` properties are dense and sequential (0, 1, 2...) before saving. - **Testing Plan:** Updated manual testing steps to reflect the new UI flow. This approach improves UX by making reordering feel instantaneous and reduces the number of API calls compared to updating on every move. --- apps/client/src/common/api/customFields.ts | 27 ++-- .../src/common/hooks-query/useCustomFields.ts | 12 +- .../panel/manage-panel/CustomFields.tsx | 134 ++++++++++++++++-- .../composite/CustomFieldEntry.tsx | 18 ++- .../custom-fields/customFields.router.ts | 34 +++-- .../custom-fields/customFields.validation.ts | 2 + .../src/api-data/rundown/rundown.dao.ts | 21 ++- .../src/api-data/rundown/rundown.service.ts | 35 +++-- .../src/definitions/core/CustomFields.type.ts | 1 + 9 files changed, 224 insertions(+), 60 deletions(-) diff --git a/apps/client/src/common/api/customFields.ts b/apps/client/src/common/api/customFields.ts index 02ce4d15e..e39df8c70 100644 --- a/apps/client/src/common/api/customFields.ts +++ b/apps/client/src/common/api/customFields.ts @@ -1,38 +1,45 @@ import axios from 'axios'; -import { CustomField, CustomFieldKey, CustomFields } from 'ontime-types'; +import { CustomField, CustomFieldKey } from 'ontime-types'; // Removed CustomFields import { apiEntryUrl } from './constants'; +// Define CustomFieldWithKey for client-side usage +export type CustomFieldWithKey = CustomField & { key: CustomFieldKey }; + const customFieldsPath = `${apiEntryUrl}/custom-fields`; /** - * Requests list of known custom fields + * Requests list of known custom fields, sorted by order */ -export async function getCustomFields(): Promise { - const res = await axios.get(customFieldsPath); +export async function getCustomFields(): Promise { + const res = await axios.get(customFieldsPath); return res.data; } /** * Sets list of known custom fields + * Returns the updated list, sorted by order */ -export async function postCustomField(newField: CustomField): Promise { - const res = await axios.post(customFieldsPath, { ...newField }); +export async function postCustomField(newField: CustomField): Promise { + const res = await axios.post(customFieldsPath, { ...newField }); return res.data; } /** * Edits single custom field + * Returns the updated list, sorted by order */ -export async function editCustomField(key: CustomFieldKey, newField: CustomField): Promise { - const res = await axios.put(`${customFieldsPath}/${key}`, { ...newField }); +export async function editCustomField(key: CustomFieldKey, newField: Partial): Promise { + // Ensure newField can include 'order' by using Partial + const res = await axios.put(`${customFieldsPath}/${key}`, { ...newField }); return res.data; } /** * Deletes single custom field + * Returns the updated list, sorted by order */ -export async function deleteCustomField(key: CustomFieldKey): Promise { - const res = await axios.delete(`${customFieldsPath}/${key}`); +export async function deleteCustomField(key: CustomFieldKey): Promise { + const res = await axios.delete(`${customFieldsPath}/${key}`); return res.data; } diff --git a/apps/client/src/common/hooks-query/useCustomFields.ts b/apps/client/src/common/hooks-query/useCustomFields.ts index 36a087fb9..0f97d0803 100644 --- a/apps/client/src/common/hooks-query/useCustomFields.ts +++ b/apps/client/src/common/hooks-query/useCustomFields.ts @@ -1,17 +1,17 @@ import { useQuery } from '@tanstack/react-query'; -import { CustomFields } from 'ontime-types'; - +// CustomFields record type is no longer used here import { queryRefetchIntervalSlow } from '../../ontimeConfig'; import { CUSTOM_FIELDS } from '../api/constants'; -import { getCustomFields } from '../api/customFields'; +import { getCustomFields, CustomFieldWithKey } from '../api/customFields'; // Import CustomFieldWithKey -const placeholder: CustomFields = {}; +const placeholder: CustomFieldWithKey[] = []; // Placeholder is now an empty array export default function useCustomFields() { - const { data, status, isFetching, isError, refetch } = useQuery({ + // Explicitly type the useQuery hook + const { data, status, isFetching, isError, refetch } = useQuery({ queryKey: CUSTOM_FIELDS, queryFn: getCustomFields, - placeholderData: (previousData, _previousQuery) => previousData, + placeholderData: (previousData, _previousQuery) => previousData ?? placeholder, retry: 5, retryDelay: (attempt) => attempt * 2500, refetchInterval: queryRefetchIntervalSlow, diff --git a/apps/client/src/features/app-settings/panel/manage-panel/CustomFields.tsx b/apps/client/src/features/app-settings/panel/manage-panel/CustomFields.tsx index 764d7fa1a..7b4274824 100644 --- a/apps/client/src/features/app-settings/panel/manage-panel/CustomFields.tsx +++ b/apps/client/src/features/app-settings/panel/manage-panel/CustomFields.tsx @@ -1,8 +1,9 @@ -import { useState } from 'react'; +import { useState, useEffect } from 'react'; // Import useEffect import { IoAdd } from 'react-icons/io5'; import { CustomField, CustomFieldKey } from 'ontime-types'; -import { deleteCustomField, editCustomField, postCustomField } from '../../../../common/api/customFields'; +// Import CustomFieldWithKey +import { deleteCustomField, editCustomField, postCustomField, CustomFieldWithKey } from '../../../../common/api/customFields'; import Button from '../../../../common/components/buttons/Button'; import Info from '../../../../common/components/info/Info'; import ExternalLink from '../../../../common/components/link/external-link/ExternalLink'; @@ -14,8 +15,61 @@ import CustomFieldEntry from './composite/CustomFieldEntry'; import CustomFieldForm from './composite/CustomFieldForm'; export default function CustomFieldSettings() { - const { data, refetch } = useCustomFields(); + const { data, refetch } = useCustomFields(); // data is CustomFieldWithKey[] const [isAdding, setIsAdding] = useState(false); + const [displayedFields, setDisplayedFields] = useState([]); + const [orderChanged, setOrderChanged] = useState(false); // To track if order has changed for enabling Save button + const [isSavingOrder, setIsSavingOrder] = useState(false); // For Save Order button loading state + + useEffect(() => { + // Initialize displayedFields with fetched data, ensuring a fresh copy for local manipulation + // And sort it initially, as the backend already provides it sorted, but this ensures consistency. + setDisplayedFields(data ? [...data].sort((a, b) => (a.order ?? Infinity) - (b.order ?? Infinity)) : []); + // When data is refetched from server (e.g. after save), reset orderChanged flag + setOrderChanged(false); + }, [data]); + + const handleSaveOrder = async () => { + setIsSavingOrder(true); + const updatePromises = []; + + // Create a map of original orders for quick lookup + const originalFieldsMap = new Map(data.map(f => [f.key, f])); + + for (let i = 0; i < displayedFields.length; i++) { + const currentField = displayedFields[i]; + const originalField = originalFieldsMap.get(currentField.key); + + // The new order is its current index in the displayedFields array + const newOrder = i; + + // Check if the effective order has changed, or if it's a new field without an original order yet (though create handles initial order) + // or if the field itself is new and not in originalFieldsMap (less likely here as it should have been created) + if (!originalField || originalField.order !== newOrder) { + // Only update if the order property is actually different + // We need to ensure the object passed to editCustomField has all required CustomField props + // and the order. The 'key' is passed as the first argument to editCustomField. + const fieldToSave: Partial = { + label: currentField.label, + type: currentField.type, + colour: currentField.colour, + order: newOrder, + }; + updatePromises.push(editCustomField(currentField.key, fieldToSave)); + } + } + + try { + await Promise.all(updatePromises); + setOrderChanged(false); + refetch(); // Refetch data from the server to confirm and get fresh state + } catch (error) { + console.error("Error saving custom field order:", error); + // Potentially show an error message to the user + } finally { + setIsSavingOrder(false); + } + }; const handleInitiateCreate = () => { setIsAdding(true); @@ -50,9 +104,14 @@ export default function CustomFieldSettings() { Custom fields - + + + + @@ -77,16 +136,67 @@ export default function CustomFieldSettings() { - {Object.entries(data).map(([key, { colour, label, type }]) => { + {displayedFields.map((field, index) => { // Iterate over displayedFields return ( { + const newFields = [...displayedFields]; + const fieldToMove = newFields[index]; + let neighborIndex = -1; + + if (direction === 'up' && index > 0) { + neighborIndex = index - 1; + } else if (direction === 'down' && index < newFields.length - 1) { + neighborIndex = index + 1; + } + + if (neighborIndex !== -1) { + const neighborField = newFields[neighborIndex]; + + // Swap order properties + const tempOrder = fieldToMove.order; + fieldToMove.order = neighborField.order; + neighborField.order = tempOrder; + + // Actual swap in the array for immediate UI feedback before potential sort + newFields[index] = neighborField; + newFields[neighborIndex] = fieldToMove; + + // Sort by the new order values to ensure dense packing if orders were sparse or undefined + // This also handles cases where initial orders might not be perfectly sequential. + newFields.sort((a, b) => (a.order ?? Infinity) - (b.order ?? Infinity)); + + // If orders became non-dense or undefined, re-assign dense orders + // This ensures that when we save, we send a clean, sequential order. + let orderIsDirty = false; + newFields.forEach((f, i) => { + if(f.order !== i) { + f.order = i; + orderIsDirty = true; + } + }); + + // If we had to re-assign orders, sort again just in case (though should be sorted) + if(orderIsDirty) { + newFields.sort((a, b) => (a.order ?? Infinity) - (b.order ?? Infinity)); + } + + + setDisplayedFields(newFields); + setOrderChanged(true); + } + }} /> ); })} diff --git a/apps/client/src/features/app-settings/panel/manage-panel/composite/CustomFieldEntry.tsx b/apps/client/src/features/app-settings/panel/manage-panel/composite/CustomFieldEntry.tsx index 629df80d8..3a6399908 100644 --- a/apps/client/src/features/app-settings/panel/manage-panel/composite/CustomFieldEntry.tsx +++ b/apps/client/src/features/app-settings/panel/manage-panel/composite/CustomFieldEntry.tsx @@ -1,5 +1,5 @@ import { useState } from 'react'; -import { IoPencil, IoTrash } from 'react-icons/io5'; +import { IoArrowDown, IoArrowUp, IoPencil, IoTrash } from 'react-icons/io5'; import { CustomField, CustomFieldKey } from 'ontime-types'; import IconButton from '../../../../../common/components/buttons/IconButton'; @@ -17,15 +17,19 @@ interface CustomFieldEntryProps { label: string; fieldKey: string; type: 'string' | 'image'; - onEdit: (key: CustomFieldKey, patch: CustomField) => Promise; + order?: number; // Add order + onEdit: (key: CustomFieldKey, patch: Partial) => Promise; // Patch can be partial for order updates onDelete: (key: CustomFieldKey) => Promise; + isFirst: boolean; + isLast: boolean; + onMove: (direction: 'up' | 'down') => void; // Changed from Promise to void } export default function CustomFieldEntry(props: CustomFieldEntryProps) { - const { colour, label, fieldKey, type, onEdit, onDelete } = props; + const { colour, label, fieldKey, type, onEdit, onDelete, isFirst, isLast, onMove } = props; const [isEditing, setIsEditing] = useState(false); - const handleEdit = async (patch: CustomField) => { + const handleEdit = async (patch: CustomField) => { // This patch comes from CustomFieldForm, so it's a full CustomField await onEdit(fieldKey, patch); setIsEditing(false); }; @@ -61,6 +65,12 @@ export default function CustomFieldEntry(props: CustomFieldEntryProps) { + onMove('up')} disabled={isFirst}> + + + onMove('down')} disabled={isLast}> + + setIsEditing(true)}> diff --git a/apps/server/src/api-data/custom-fields/customFields.router.ts b/apps/server/src/api-data/custom-fields/customFields.router.ts index 7de1de47e..eb76792b4 100644 --- a/apps/server/src/api-data/custom-fields/customFields.router.ts +++ b/apps/server/src/api-data/custom-fields/customFields.router.ts @@ -1,47 +1,51 @@ -import { CustomField, CustomFields, ErrorResponse } from 'ontime-types'; +import { CustomField, ErrorResponse } from 'ontime-types'; import { getErrorMessage } from 'ontime-utils'; import express from 'express'; import type { Request, Response } from 'express'; -import { getProjectCustomFields } from '../rundown/rundown.dao.js'; +import { getProjectCustomFields, CustomFieldWithKey } from '../rundown/rundown.dao.js'; import { createCustomField, editCustomField, deleteCustomField } from '../rundown/rundown.service.js'; import { validateCustomField, validateDeleteCustomField, validateEditCustomField } from './customFields.validation.js'; export const router = express.Router(); -router.get('/', async (_req: Request, res: Response) => { - const customFields = getProjectCustomFields(); - res.status(200).json(customFields); +// Response type changed to CustomFieldWithKey[] +router.get('/', async (_req: Request, res: Response) => { + const customFieldsArray = getProjectCustomFields(); + res.status(200).json(customFieldsArray); }); -router.post('/', validateCustomField, async (req: Request, res: Response) => { +// The POST, PUT, DELETE operations in this router return the entire updated CustomFields object. +// This will also need to change to return the sorted array if we want consistency. +router.post('/', validateCustomField, async (req: Request, res: Response) => { try { - const newFields = await createCustomField(req.body as CustomField); - res.status(201).send(newFields); + // req.body will include label, type, colour, and optionally order + const newFieldsArray = await createCustomField(req.body as CustomField); + res.status(201).send(newFieldsArray); } catch (error) { const message = getErrorMessage(error); res.status(400).send({ message }); } }); -router.put('/:key', validateEditCustomField, async (req: Request, res: Response) => { +router.put('/:key', validateEditCustomField, async (req: Request, res: Response) => { try { const currentKey = req.params.key; - const { colour, type, label } = req.body; - const newFields = await editCustomField(currentKey, { label, colour, type }); - res.status(200).send(newFields); + const { colour, type, label, order } = req.body; // order is now included + const newFieldsArray = await editCustomField(currentKey, { label, colour, type, order }); + res.status(200).send(newFieldsArray); } catch (error) { const message = getErrorMessage(error); res.status(400).send({ message }); } }); -router.delete('/:key', validateDeleteCustomField, async (req: Request, res: Response) => { +router.delete('/:key', validateDeleteCustomField, async (req: Request, res: Response) => { try { - const customFields = await deleteCustomField(req.params.key); - res.status(200).send(customFields); + const customFieldsArray = await deleteCustomField(req.params.key); + res.status(200).send(customFieldsArray); } catch (error) { const message = getErrorMessage(error); res.status(400).send({ message }); diff --git a/apps/server/src/api-data/custom-fields/customFields.validation.ts b/apps/server/src/api-data/custom-fields/customFields.validation.ts index f63250919..5dd8cffdb 100644 --- a/apps/server/src/api-data/custom-fields/customFields.validation.ts +++ b/apps/server/src/api-data/custom-fields/customFields.validation.ts @@ -13,6 +13,7 @@ export const validateCustomField = [ }), body('type').isIn(['string', 'image']), body('colour').isString().trim(), + body('order').optional().isNumeric().toInt(), requestValidationFunction, ]; @@ -28,6 +29,7 @@ export const validateEditCustomField = [ }), body('type').isIn(['string', 'image']), body('colour').isString().trim(), + body('order').optional().isNumeric().toInt(), requestValidationFunction, ]; diff --git a/apps/server/src/api-data/rundown/rundown.dao.ts b/apps/server/src/api-data/rundown/rundown.dao.ts index a7209247f..5a95ecc02 100644 --- a/apps/server/src/api-data/rundown/rundown.dao.ts +++ b/apps/server/src/api-data/rundown/rundown.dao.ts @@ -82,7 +82,21 @@ let projectCustomFields: CustomFields = {}; export const getCurrentRundown = (): Readonly => cachedRundown; export const getRundownMetadata = (): Readonly => rundownMetadata; -export const getProjectCustomFields = (): Readonly => projectCustomFields; + +export type CustomFieldWithKey = CustomField & { key: CustomFieldKey }; +export const getProjectCustomFields = (): ReadonlyArray => { + return Object.entries(projectCustomFields) + .map(([key, value]): CustomFieldWithKey => ({ key, ...value })) + .sort((a, b) => { + const orderA = a.order ?? Infinity; // Fields without order go last + const orderB = b.order ?? Infinity; + if (orderA === orderB) { + // Fallback sort by key if order is the same or both undefined + return a.key.localeCompare(b.key); + } + return orderA - orderB; + }); +}; export const getEntryWithId = (entryId: EntryId): OntimeEntry | undefined => cachedRundown.entries[entryId]; type Transaction = { @@ -563,9 +577,12 @@ function customFieldAdd(customFields: CustomFields, key: CustomFieldKey, newCust label: newCustomField.label, type: newCustomField.type, colour: newCustomField.colour, + order: newCustomField.order, // Add order here }; - return { [key]: newCustomField }; + // The return value of this function doesn't seem to be critically used for its content, + // but to be safe, let's include order here too. + return { [key]: { ...newCustomField } }; } /** diff --git a/apps/server/src/api-data/rundown/rundown.service.ts b/apps/server/src/api-data/rundown/rundown.service.ts index 3872a8b2b..2db2de18b 100644 --- a/apps/server/src/api-data/rundown/rundown.service.ts +++ b/apps/server/src/api-data/rundown/rundown.service.ts @@ -19,7 +19,7 @@ import { customFieldLabelToKey } from 'ontime-utils'; import { updateRundownData } from '../../stores/runtimeState.js'; import { runtimeService } from '../../services/runtime-service/RuntimeService.js'; -import { createTransaction, customFieldMutation, rundownCache, rundownMutation } from './rundown.dao.js'; +import { createTransaction, customFieldMutation, rundownCache, rundownMutation, getProjectCustomFields, CustomFieldWithKey } from './rundown.dao.js'; import type { RundownMetadata } from './rundown.types.js'; import { generateEvent, getInsertAfterId, hasChanges } from './rundown.utils.js'; import { sendRefetch } from '../../adapters/WebsocketAdapter.js'; @@ -412,7 +412,7 @@ export async function ungroupEntries(blockId: EntryId): Promise { * Adds a new custom field to the project * @throws if the label is missing or invalid */ -export async function createCustomField(customField: CustomField): Promise { +export async function createCustomField(customField: CustomField): Promise> { const key = customFieldLabelToKey(customField.label); if (!key) { @@ -426,16 +426,27 @@ export async function createCustomField(customField: CustomField): Promise { + if (cf.order !== undefined && cf.order > maxOrder) { + maxOrder = cf.order; + } + }); + customField.order = maxOrder + 1; + } + customFieldMutation.add(customFields, key, customField); // Adding a custom field has no immediate implications on the rundown - const { customFields: resultCustomFields } = commit(false); + commit(false); // Persist changes setImmediate(() => { sendRefetch(RefetchKey.CustomFields); }); - return resultCustomFields; + return getProjectCustomFields(); // Return sorted array } /** @@ -446,7 +457,7 @@ export async function createCustomField(customField: CustomField): Promise): Promise { +export async function editCustomField(key: CustomFieldKey, newField: Partial): Promise> { const { customFields, customFieldsMetadata, rundown, commit } = createTransaction({ mutableRundown: true, mutableCustomFields: true, @@ -470,26 +481,27 @@ export async function editCustomField(key: CustomFieldKey, newField: Partial { notifyChanges(rundownMetadata, revision, { timer: true, external: true }); + sendRefetch(RefetchKey.CustomFields); // Also refetch custom fields explicitly }); - return resultCustomFields; + return getProjectCustomFields(); // Return sorted array } /** * Deletes an existing custom field */ -export async function deleteCustomField(key: CustomFieldKey): Promise { +export async function deleteCustomField(key: CustomFieldKey): Promise> { const { customFields, customFieldsMetadata, rundown, commit } = createTransaction({ mutableRundown: true, mutableCustomFields: true, }); if (!(key in customFields)) { - return customFields; + return getProjectCustomFields(); // Return sorted array even if key not found } customFieldMutation.remove(customFields, key); @@ -498,14 +510,15 @@ export async function deleteCustomField(key: CustomFieldKey): Promise { notifyChanges(rundownMetadata, revision, { timer: true, external: true }); + sendRefetch(RefetchKey.CustomFields); // Also refetch custom fields explicitly }); - return resultCustomFields; + return getProjectCustomFields(); // Return sorted array } /** diff --git a/packages/types/src/definitions/core/CustomFields.type.ts b/packages/types/src/definitions/core/CustomFields.type.ts index 3f8333fb4..05a5cac69 100644 --- a/packages/types/src/definitions/core/CustomFields.type.ts +++ b/packages/types/src/definitions/core/CustomFields.type.ts @@ -4,6 +4,7 @@ export type CustomField = { type: 'string' | 'image'; colour: string; label: string; + order?: number; }; export type CustomFields = Record;