From dda16fd5c9298183f96296db329bb55a567bb99c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sindri=20=C3=9E=C3=B3r?= <70705172+SindriTh@users.noreply.github.com> Date: Tue, 21 Nov 2023 09:58:11 +0000 Subject: [PATCH] Improve Performance of DBus Interface Handling and ListItem Rendering (#270) * Added memo around ListItem * Removed history * Rewrite Demo * Subscription handling rewrite * Fixed number tinker --- .../IPC/src/Components/IOList/ListItem.tsx | 126 ++---- .../IPC/src/Components/Tinker/BoolTinker.tsx | 35 +- .../IPC/src/Components/Tinker/JSONTinker.tsx | 37 +- .../src/Components/Tinker/NumberTinker.tsx | 54 ++- .../src/Components/Tinker/StringTinker.tsx | 32 +- cockpit/IPC/src/views/IODebug.css | 12 +- cockpit/IPC/src/views/IODebug.tsx | 393 ++++++------------ cockpit/IPC/src/views/ListDBUS.tsx | 8 + cockpit/IPC/src/views/StateMachine.tsx | 1 - 9 files changed, 299 insertions(+), 399 deletions(-) diff --git a/cockpit/IPC/src/Components/IOList/ListItem.tsx b/cockpit/IPC/src/Components/IOList/ListItem.tsx index 34fcc71c64..aa191e6539 100644 --- a/cockpit/IPC/src/Components/IOList/ListItem.tsx +++ b/cockpit/IPC/src/Components/IOList/ListItem.tsx @@ -4,37 +4,29 @@ import React, { ChangeEvent, ReactElement } from 'react'; import { ClipboardCopy, ClipboardCopyVariant, - DataListAction, DataListCell, DataListItem, DataListItemCells, DataListItemRow, - Dropdown, - DropdownItem, - DropdownList, - MenuToggleElement, Tooltip, } from '@patternfly/react-core'; import Circle from 'src/Components/Simple/Circle'; import { removeSlotOrg } from 'src/Components/Form/WidgetFunctions'; -import CustomMenuToggle from 'src/Components/Dropdown/CustomMenuToggle'; import StringTinker from 'src/Components/Tinker/StringTinker'; import BoolTinker from 'src/Components/Tinker/BoolTinker'; import NumberTinker from 'src/Components/Tinker/NumberTinker'; interface ListItemProps { dbusInterface: any, - index: number, - activeDropdown: number | null, - dropdownRefs: any, - onToggleClick: any, - setModalOpen: any, onCheck: (checked: boolean) => void, + isChecked: boolean, + data: any, } // eslint-disable-next-line react/function-component-definition const ListItem: React.FC = ({ - dbusInterface, index, activeDropdown, dropdownRefs, onToggleClick, setModalOpen, onCheck, + dbusInterface, + onCheck, isChecked, data, }) => { const isMobile = window.innerWidth < 768; /** @@ -42,15 +34,17 @@ const ListItem: React.FC = ({ * @param data The data to be displayed * @returns ReactElement to be displayed */ - function handleBoolContent(data: any): ReactElement { + function handleBoolContent(booldata: any): ReactElement { return ( - + { isChecked && booldata !== null + ? + : } ); } @@ -60,10 +54,11 @@ const ListItem: React.FC = ({ * @param data The data to be displayed * @returns ReactElement to be displayed */ - function handleStringContent(data: any): ReactElement { - return ( -

{data.Value}

- ); + function handleStringContent(stringdata: any): ReactElement { + if (!isChecked || stringdata === null) { + return (

Unknown

); + } + return (

{stringdata}

); } /** @@ -71,12 +66,16 @@ const ListItem: React.FC = ({ * @param data The data to be displayed * @returns ReactElement to be displayed */ - function handleJSONContent(data: any): ReactElement { + function handleJSONContent(stringdata: any): ReactElement { + if (!isChecked || stringdata === null) { + return (

Unknown

); + } + // Test if string is JSON let isJSON = false; - if (data.Value[0] === '{' || data.Value[0] === '[') { + if (stringdata[0] === '{' || stringdata[0] === '[') { try { - JSON.parse(data.Value); + JSON.parse(stringdata); isJSON = true; } catch { isJSON = false; @@ -92,16 +91,16 @@ const ListItem: React.FC = ({ clickTip="Copied" variant={ClipboardCopyVariant.expansion} style={{ - alignSelf: 'baseline', marginTop: '-.85rem', zIndex: 1, maxWidth: '25vw', + alignSelf: 'baseline', marginTop: '-.25rem', zIndex: 1, maxWidth: '25vw', }} > - {JSON.stringify(JSON.parse(data.Value), null, 2)} + {JSON.stringify(JSON.parse(stringdata), null, 2)} ); } return ( -

{data.Value}

+

{stringdata}

); } @@ -110,23 +109,23 @@ const ListItem: React.FC = ({ * @param data Interface data * @returns ReactElement to be displayed */ - function getSecondaryContent(data: any): ReactElement | null { - const internals = (interfacedata: any) => { - switch (interfacedata.type) { + function getSecondaryContent(interfaceData: any): ReactElement | null { + const internals = (secData: any) => { + switch (secData.type) { case 'boolean': - return handleBoolContent(interfacedata.proxy.data); + return handleBoolContent(data); case 'string': - return handleJSONContent(interfacedata.proxy.data); + return handleJSONContent(data); // If the string is not JSON, it falls back to string case 'object': - return handleJSONContent(interfacedata.proxy.data); + return handleJSONContent(data); case 'number': case 'integer': case 'double': - return handleStringContent(interfacedata.proxy.data); + return handleStringContent(data); default: return <>Type Error; @@ -142,7 +141,7 @@ const ListItem: React.FC = ({ padding: isMobile ? '10px' : '0px', }} > - {internals(data)} + {internals(interfaceData)} ); } @@ -152,22 +151,22 @@ const ListItem: React.FC = ({ * @param data Interface data * @returns ReactElement to be displayed */ - function getTinkerInterface(data: any): React.ReactElement | null { + function getTinkerInterface(interfaceData: any): React.ReactElement | null { const internals = (interfacedata: any) => { switch (interfacedata.type) { case 'boolean': - return ; + return ; case 'string': - return ; + return ; case 'object': - return ; + return ; case 'number': case 'integer': case 'double': - return ; + return ; default: return <>Type Error; @@ -181,35 +180,34 @@ const ListItem: React.FC = ({ justifyContent: 'start', }} > - {internals(data)} + {internals(interfaceData)} ); } - const onSelect = (_event: React.MouseEvent | undefined, value: string | number | undefined) => { - console.log(value); - }; const handleCheckboxChange = (event: ChangeEvent) => { onCheck(event.target.checked); }; return ( - + -

{removeSlotOrg(dbusInterface.proxy.iface)}

+

{removeSlotOrg(dbusInterface.interfaceName)}

, = ({ ? ( = ({ : null, ]} /> - - ) => ( // NOSONAR - { - if (typeof toggleRef === 'function') { - toggleRef(ref); - } - dropdownRefs.current[index] = ref; // Store the ref for the dropdown - }} - onClick={() => onToggleClick(index)} - isExpanded={dbusInterface.dropdown} - /> - )} - isOpen={activeDropdown === index} - onSelect={onSelect} - popperProps={{ enableFlip: true }} - > - - View History - setModalOpen(index)} - > - Watch - - - -
); diff --git a/cockpit/IPC/src/Components/Tinker/BoolTinker.tsx b/cockpit/IPC/src/Components/Tinker/BoolTinker.tsx index b43a69d72a..750949ba1d 100644 --- a/cockpit/IPC/src/Components/Tinker/BoolTinker.tsx +++ b/cockpit/IPC/src/Components/Tinker/BoolTinker.tsx @@ -1,32 +1,41 @@ /* eslint-disable react/function-component-definition */ import React from 'react'; +import { TFC_DBUS_DOMAIN, TFC_DBUS_ORGANIZATION } from 'src/variables'; import { AlertVariant, Switch } from '@patternfly/react-core'; import { useAlertContext } from '../Alert/AlertContext'; interface BoolTinkerIface { - data: any + interfaceData: any; + isChecked: boolean; } -const BoolTinker: React.FC = ({ data }) => { +const BoolTinker: React.FC = ({ interfaceData, isChecked }) => { const { addAlert } = useAlertContext(); - const [value, setValue] = React.useState(data.proxy.data.Value); + const [value, setValue] = React.useState(interfaceData.data); + const slotPath = `/${TFC_DBUS_DOMAIN}/${TFC_DBUS_ORGANIZATION}/Slots`; + const signalPath = `/${TFC_DBUS_DOMAIN}/${TFC_DBUS_ORGANIZATION}/Signals`; - const handleInputChange = (newValue: boolean) => { - data.proxy.Tinker(newValue ? 1 : 0).then(() => { - addAlert(`Value of ${data.interfaceName} has been set ${newValue ? 'true' : 'false'}`, AlertVariant.success); - setValue(newValue); - }).catch(() => { - addAlert(`Error setting value of ${data.interfaceName}`, AlertVariant.danger); + const handleInputChange = async (newValue: boolean) => { + const client = window.cockpit.dbus(interfaceData.process, { bus: 'system', superuser: 'try' }); + const proxy = client.proxy(interfaceData.interfaceName, interfaceData.direction === 'slot' ? slotPath : signalPath); + await proxy.wait().then(() => { + proxy.Tinker(newValue ? 1 : 0).then(() => { + addAlert(`Value of ${interfaceData.interfaceName} has been set ${newValue ? 'true' : 'false'}`, AlertVariant.success); + setValue(newValue); + }).catch((e: any) => { + addAlert(`Error setting value of ${interfaceData.interfaceName}`, AlertVariant.danger); + console.log(e); + }); }); }; return ( handleInputChange(val)} - isDisabled={false} - key={`${data.iface}-${data.process}`} + isDisabled={!isChecked} + key={`${interfaceData.interfaceName}-${interfaceData.process}`} /> ); }; diff --git a/cockpit/IPC/src/Components/Tinker/JSONTinker.tsx b/cockpit/IPC/src/Components/Tinker/JSONTinker.tsx index 7c6aa43454..ba7998ad7e 100644 --- a/cockpit/IPC/src/Components/Tinker/JSONTinker.tsx +++ b/cockpit/IPC/src/Components/Tinker/JSONTinker.tsx @@ -4,44 +4,61 @@ import { AlertVariant, Button, ClipboardCopy, ClipboardCopyVariant, } from '@patternfly/react-core'; import { CheckIcon } from '@patternfly/react-icons'; +import { TFC_DBUS_DOMAIN, TFC_DBUS_ORGANIZATION } from 'src/variables'; import { useAlertContext } from '../Alert/AlertContext'; interface StringTinkerIface { - data: any; + interfaceData: any; + isChecked: boolean; } -const StringTinker: React.FC = ({ data }) => { - const [inputValue, setInputValue] = useState(data.proxy.data.Value); +// eslint-disable-next-line react/function-component-definition +const StringTinker: React.FC = ({ interfaceData, isChecked }) => { + const [inputValue, setInputValue] = useState(interfaceData.data); const { addAlert } = useAlertContext(); + const slotPath = `/${TFC_DBUS_DOMAIN}/${TFC_DBUS_ORGANIZATION}/Slots`; + const signalPath = `/${TFC_DBUS_DOMAIN}/${TFC_DBUS_ORGANIZATION}/Signals`; + const handleInputChange = (value: string | number | undefined) => { setInputValue(value?.toString() ?? ''); }; - function updateDBUS() { - data.proxy.Tinker(inputValue); - addAlert(`Value of ${data.interfaceName} has been set to ${inputValue}`, AlertVariant.success); + async function updateDBUS() { + const client = window.cockpit.dbus(interfaceData.process, { bus: 'system', superuser: 'try' }); + const proxy = client.proxy(interfaceData.interfaceName, interfaceData.direction === 'slot' ? slotPath : signalPath); + await proxy.wait().then(() => { + proxy.Tinker(inputValue); + }).catch((e: any) => { + addAlert(`Error setting value of ${interfaceData.interfaceName}`, AlertVariant.danger); + console.log(e); + }); + addAlert(`Value of ${interfaceData.interfaceName} has been set to ${inputValue}`, AlertVariant.success); } + if (!isChecked) { + return ( +

Unknown

); + } return ( <> handleInputChange(val)} onKeyDown={(e) => { if (e.key === 'Enter') { updateDBUS(); } }} style={{ alignSelf: 'baseline', marginTop: '-.25rem', zIndex: 1 }} - key={`${data.iface}-${data.process}`} + key={`${interfaceData.interfaceName}-${interfaceData.process}`} > - {JSON.stringify(JSON.parse(data.Value), null, 2)} + {JSON.stringify(JSON.parse(interfaceData.Value), null, 2)}