src: Addressing comments

- addressed AI comments
- fix tab indexing
- added username to the title of Remove user modal
- ensured "Invalid password" is not in the errors object before a password is filled in
- removing a user with empty user, password and ssh key fields does not prompt "Remove user" modal
- check for duplicate username
- focus on new user tab
- remove redundant tabIndex
This commit is contained in:
regexowl 2025-04-04 15:20:35 +02:00 committed by Lucas Garfield
parent 8d74022ab1
commit 6b0af16471
5 changed files with 90 additions and 22 deletions

View file

@ -2,6 +2,8 @@ import React from 'react';
import { Button, Modal } from '@patternfly/react-core';
import calculateNewIndex from './calculateNewIndex';
import { useAppDispatch, useAppSelector } from '../../../../../store/hooks';
import { removeUser, selectUsers } from '../../../../../store/wizardSlice';
@ -12,6 +14,7 @@ type RemoveUserModalProps = {
tabIndex: number;
setIndex: React.Dispatch<React.SetStateAction<number>>;
isOpen: boolean;
userName: string;
};
const RemoveUserModal = ({
@ -21,6 +24,7 @@ const RemoveUserModal = ({
tabIndex,
setIndex,
isOpen,
userName,
}: RemoveUserModalProps) => {
const dispatch = useAppDispatch();
const users = useAppSelector(selectUsers);
@ -30,16 +34,11 @@ const RemoveUserModal = ({
};
const onConfirm = () => {
const tabIndexNum = tabIndex;
let nextTabIndex = activeTabKey;
if (tabIndexNum < activeTabKey) {
// if a preceding tab is closing, keep focus on the new index of the current tab
nextTabIndex = activeTabKey - 1 > 0 ? activeTabKey - 1 : 0;
} else if (activeTabKey === users.length - 1) {
// if the closing tab is the last tab, focus the preceding tab
nextTabIndex = users.length - 2 > 0 ? users.length - 2 : 0;
}
const nextTabIndex = calculateNewIndex(
tabIndex,
activeTabKey,
users.length
);
setActiveTabKey(nextTabIndex);
setIndex(nextTabIndex);
@ -49,7 +48,7 @@ const RemoveUserModal = ({
return (
<Modal
title="Remove user?"
title={`Remove user${userName ? ` ${userName}` : ''}?`}
isOpen={isOpen}
onClose={onClose}
width="50%"

View file

@ -10,6 +10,7 @@ import {
} from '@patternfly/react-core';
import { ExternalLinkAltIcon } from '@patternfly/react-icons';
import calculateNewIndex from './calculateNewIndex';
import RemoveUserModal from './RemoveUserModal';
import { GENERATING_SSH_KEY_PAIRS_URL } from '../../../../../constants';
@ -23,6 +24,7 @@ import {
selectUsers,
addUserGroupByIndex,
removeUserGroupByIndex,
removeUser,
} from '../../../../../store/wizardSlice';
import LabelInput from '../../../LabelInput';
import { PasswordValidatedInput } from '../../../utilities/PasswordValidatedInput';
@ -37,9 +39,14 @@ const UserInfo = () => {
const [index, setIndex] = useState(0);
const [activeTabKey, setActiveTabKey] = useState(0);
const [tabIndex, setTabIndex] = useState(0);
const [showRemoveUserModal, setShowRemoveUserModal] = useState(false);
// Taken directly from PF5 Dynamic tabs documentation
// https://v5-archive.patternfly.org/components/tabs#dynamic-tabs
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const tabComponentRef = React.useRef<any>();
const firstMount = React.useRef(true);
const onSelect = (event: React.MouseEvent, tabIndex: number) => {
setActiveTabKey(tabIndex);
setIndex(tabIndex);
@ -51,9 +58,37 @@ const UserInfo = () => {
dispatch(addUser());
};
React.useEffect(() => {
if (firstMount.current) {
firstMount.current = false;
return;
} else {
const first =
tabComponentRef.current.tabList.current.childNodes[activeTabKey];
if (first) {
first.firstChild.focus();
}
}
}, [users.length]);
const onClose = (_event: React.MouseEvent, tabIndex: number) => {
setShowRemoveUserModal(true);
setTabIndex(tabIndex);
if (
users[tabIndex].name === '' &&
users[tabIndex].password === '' &&
users[tabIndex].ssh_key === ''
) {
const nextTabIndex = calculateNewIndex(
tabIndex,
activeTabKey,
users.length
);
setActiveTabKey(nextTabIndex);
setIndex(nextTabIndex);
dispatch(removeUser(tabIndex));
} else {
setShowRemoveUserModal(true);
setIndex(tabIndex);
}
};
const handleNameChange = (
@ -102,9 +137,10 @@ const UserInfo = () => {
setShowRemoveUserModal={setShowRemoveUserModal}
activeTabKey={activeTabKey}
setActiveTabKey={setActiveTabKey}
tabIndex={tabIndex}
tabIndex={index}
setIndex={setIndex}
isOpen={showRemoveUserModal}
userName={users[index] ? users[index].name : ''}
/>
<Tabs
aria-label="Users tabs"
@ -112,11 +148,12 @@ const UserInfo = () => {
onSelect={onSelect}
onAdd={onAdd}
onClose={onClose}
ref={tabComponentRef}
>
{users.map((user, index) => (
<Tab
aria-label={`User ${user.name} tab`}
key={user.name}
key={index}
eventKey={index}
title={<TabTitleText>{user.name || 'New user'}</TabTitleText>}
>
@ -167,7 +204,7 @@ const UserInfo = () => {
}
onChange={(_e, value) => handleCheckboxChange(_e, value)}
aria-label="Administrator"
id="user Administrator"
id={`${user.name}-${index}`}
name="user Administrator"
/>
</FormGroup>

View file

@ -0,0 +1,20 @@
function calculateNewIndex(
tabIndex: number,
activeTabKey: number,
usersLength: number
) {
const tabIndexNum = tabIndex;
let nextTabIndex = activeTabKey;
if (tabIndexNum < activeTabKey) {
// if a preceding tab is closing, keep focus on the new index of the current tab
nextTabIndex = activeTabKey - 1 > 0 ? activeTabKey - 1 : 0;
} else if (activeTabKey === usersLength - 1) {
// if the closing tab is the last tab, focus the preceding tab
nextTabIndex = usersLength - 2 >= 0 ? usersLength - 2 : 0;
}
return nextTabIndex;
}
export default calculateNewIndex;

View file

@ -31,6 +31,7 @@ import {
selectSatelliteCaCertificate,
selectSatelliteRegistrationCommand,
selectImageTypes,
UserWithAdditionalInfo,
} from '../../../store/wizardSlice';
import { keyboardsList } from '../steps/Locale/keyboardsList';
import { languagesList } from '../steps/Locale/languagesList';
@ -451,13 +452,23 @@ export function useServicesValidation(): StepValidation {
};
}
const validateUserName = (userName: string): string => {
const validateUserName = (
users: UserWithAdditionalInfo[],
userName: string
): string => {
if (!userName) {
return 'Required value';
}
if (userName && !isUserNameValid(userName)) {
if (!isUserNameValid(userName)) {
return 'Invalid user name';
}
// check for duplicate names
const duplicateName =
new Set(users.map((user) => user.name)).size !== users.length;
if (duplicateName) {
return 'Username already exists';
}
return '';
};
@ -481,13 +492,14 @@ export function useUsersValidation(): UsersStepValidation {
}
for (let index = 0; index < users.length; index++) {
const userNameError = validateUserName(users[index].name);
const userNameError = validateUserName(users, users[index].name);
const sshKeyError = validateSshKey(users[index].ssh_key);
const isPasswordValid = checkPasswordValidity(
users[index].password,
environments.includes('azure')
).isValid;
const passwordError = !isPasswordValid ? 'Invalid password' : '';
const passwordError =
users[index].password && !isPasswordValid ? 'Invalid password' : '';
if (
userNameError ||