Skip to content

Commit

Permalink
Minor UI bugs (#2063)
Browse files Browse the repository at this point in the history
This pull request includes a variety of changes across multiple files to
improve the functionality and user experience of the web application.
The most important changes include updates to action labels,
enhancements to drawer components, and improvements to form validation
and input handling.

### Action Labels:
* Changed the label for the latency action to "Latency Sampler" in
`frontend/webapp/containers/main/actions/action-modal/action-options.ts`
to better reflect its functionality.

### Drawer Enhancements:
* Added a delete button with a `TrashIcon` to the `DrawerHeader`
component and updated the `OverviewDrawer` to handle delete actions.
[[1]](diffhunk://#diff-2410bbb07cf40a69a4bc6a34db093cdfcc2cb9d4b98b144a37a2c6a3e1a511ffR2-R4)
[[2]](diffhunk://#diff-2410bbb07cf40a69a4bc6a34db093cdfcc2cb9d4b98b144a37a2c6a3e1a511ffR64-R68)
[[3]](diffhunk://#diff-2410bbb07cf40a69a4bc6a34db093cdfcc2cb9d4b98b144a37a2c6a3e1a511ffL96-R115)
[[4]](diffhunk://#diff-732721edf7c32d883ffefa52d6e43339c6ee328c88c3cb5f0d87f6760347b599R117-R118)

### Form Validation:
* Improved validation for action forms by adding checks for required
fields and specific validation for latency sampler endpoints in
`useActionFormData`.
[[1]](diffhunk://#diff-034891eac1659418ed1ceccc6d28858abafa1f6f5da688a2550123de5ee99841L1-R4)
[[2]](diffhunk://#diff-034891eac1659418ed1ceccc6d28858abafa1f6f5da688a2550123de5ee99841R28-R52)

### Input Handling:
* Removed unnecessary `handleKeyDown` methods and improved input
handling across various reusable components such as `InputList`,
`InputTable`, and `KeyValueInputsList`.
[[1]](diffhunk://#diff-acee2ab5bcea995f24ca47fd5f34b51d1a222c0b839ea863f0bdc2d2ac5824dcL74-R74)
[[2]](diffhunk://#diff-a00687e99957ec5d2b28432fc1959a9819b6f87371692a0bef21cf567bea7bfdL106-L109)
[[3]](diffhunk://#diff-a00687e99957ec5d2b28432fc1959a9819b6f87371692a0bef21cf567bea7bfdL121-R117)
[[4]](diffhunk://#diff-511aed49818334957716043c1273ee4e2a4c6bdc789f95e3cc2958f4cf603a1eL107-L110)
[[5]](diffhunk://#diff-511aed49818334957716043c1273ee4e2a4c6bdc789f95e3cc2958f4cf603a1eL146-R142)
[[6]](diffhunk://#diff-be0b643b1628d8036777fd4a042b1d241af86dd5035bfe9548a3b58b0dd0d327L114-L117)
[[7]](diffhunk://#diff-be0b643b1628d8036777fd4a042b1d241af86dd5035bfe9548a3b58b0dd0d327L132-R128)
[[8]](diffhunk://#diff-be0b643b1628d8036777fd4a042b1d241af86dd5035bfe9548a3b58b0dd0d327L146-R138)

### Notification Note Styling:
* Updated the styling and structure of the `NotificationNote` component
to enhance its appearance and functionality.
[[1]](diffhunk://#diff-aafb83c2f2f513f6ea10e4be1483dd18d389bb09c7708ded9b520e5b3e1d9d42L2-R8)
[[2]](diffhunk://#diff-aafb83c2f2f513f6ea10e4be1483dd18d389bb09c7708ded9b520e5b3e1d9d42R57-R67)
[[3]](diffhunk://#diff-aafb83c2f2f513f6ea10e4be1483dd18d389bb09c7708ded9b520e5b3e1d9d42L75-R92)
[[4]](diffhunk://#diff-aafb83c2f2f513f6ea10e4be1483dd18d389bb09c7708ded9b520e5b3e1d9d42L134-L142)
  • Loading branch information
BenElferink authored Dec 24, 2024
1 parent 94e1e05 commit 1cd219d
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 141 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export const ACTION_OPTIONS: ActionOption[] = [
},
{
id: 'latency-action',
label: 'Latency Action',
label: 'Latency Sampler',
description: 'Add latency to your traces.',
type: ActionsType.LATENCY_SAMPLER,
icon: getActionIcon('sampler'),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,12 @@ export function AddDestinationContainer() {
await createSources(configuredSources, configuredFutureApps);
await Promise.all(configuredDestinations.map(async ({ form }) => await createDestination(form)));

resetState();
router.push(ROUTES.OVERVIEW);
// Delay redirect by 3 seconds to allow the sources to be created on the backend 1st,
// otherwise we would have to apply polling on the overview page on every mount.
setTimeout(() => {
resetState();
router.push(ROUTES.OVERVIEW);
}, 3000);
};

const isSourcesListEmpty = () => !Object.values(configuredSources).some((sources) => !!sources.length);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const DestinationFormBody = ({ isUpdate, destination, formData, formError
const { required, value } = dynamicFields[i];

if (required) {
if (value !== undefined) {
if (![undefined, null, ''].includes(value)) {
didAutoFill = true;
} else {
didAutoFill = false;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { useEffect, useState, forwardRef, useImperativeHandle } from 'react';
import theme from '@/styles/theme';
import styled from 'styled-components';
import { EditIcon, SVG, XIcon } from '@/assets';
import { EditIcon, SVG, TrashIcon, XIcon } from '@/assets';
import { Button, IconWrapped, Input, Text, Tooltip } from '@/reuseable-components';

const HeaderContainer = styled.section`
Expand Down Expand Up @@ -60,9 +61,11 @@ interface DrawerHeaderProps {
isEdit?: boolean;
onEdit?: () => void;
onClose: () => void;
onDelete?: () => void;
deleteLabel?: string;
}

const DrawerHeader = forwardRef<DrawerHeaderRef, DrawerHeaderProps>(({ title, titleTooltip, icon, iconSrc, isEdit, onEdit, onClose }, ref) => {
const DrawerHeader = forwardRef<DrawerHeaderRef, DrawerHeaderProps>(({ title, titleTooltip, icon, iconSrc, isEdit, onEdit, onClose, onDelete, deleteLabel = 'Delete' }, ref) => {
const [inputValue, setInputValue] = useState(title);

useEffect(() => {
Expand Down Expand Up @@ -93,14 +96,23 @@ const DrawerHeader = forwardRef<DrawerHeaderRef, DrawerHeaderProps>(({ title, ti
</InputWrapper>
)}

<SectionItemsWrapper $gap={8}>
{!isEdit && !!onEdit && (
<SectionItemsWrapper $gap={2}>
{!!onEdit && !isEdit && (
<EditButton data-id='drawer-edit' variant='tertiary' onClick={onEdit}>
<EditIcon />
<ButtonText>Edit</ButtonText>
</EditButton>
)}

{!!onDelete && !isEdit && (
<EditButton data-id='drawer-delete' variant='tertiary' onClick={onDelete}>
<TrashIcon />
<Text color={theme.text.error} size={14} family='secondary'>
{deleteLabel}
</Text>
</EditButton>
)}

<CloseButton data-id='drawer-close' variant='secondary' onClick={onClose}>
<XIcon size={12} />
</CloseButton>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ const OverviewDrawer: React.FC<Props & PropsWithChildren> = ({ children, title,
isEdit={isEdit}
onEdit={onEdit ? () => onEdit(true) : undefined}
onClose={isEdit ? clickCancel : closeDrawer}
onDelete={onEdit ? clickDelete : undefined}
deleteLabel={isSource ? 'Uninstrument' : undefined}
/>
<ContentArea>{children}</ContentArea>
<DrawerFooter isOpen={isEdit} onSave={clickSave} onCancel={clickCancel} onDelete={clickDelete} deleteLabel={isSource ? 'Uninstrument' : undefined} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const SourceDrawer: React.FC<Props> = () => {
if (!selectedItem) return [];

const { item } = selectedItem as { item: K8sActualSource };
if (!item?.instrumentedApplicationDetails?.conditions) return [];

const hasPresenceOfOtherAgent = item.instrumentedApplicationDetails.conditions.some(
(condition) => condition.status === BACKEND_BOOLEAN.FALSE && condition.message.includes('device not added to any container due to the presence of another agent'),
Expand Down
21 changes: 17 additions & 4 deletions frontend/webapp/hooks/actions/useActionFormData.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { DrawerItem, useNotificationStore } from '@/store';
import { FORM_ALERTS } from '@/utils';
import { useGenericForm } from '@/hooks';
import { NOTIFICATION_TYPE, type ActionDataParsed, type ActionInput } from '@/types';
import { FORM_ALERTS, safeJsonParse } from '@/utils';
import { DrawerItem, useNotificationStore } from '@/store';
import { ActionsType, LatencySamplerSpec, NOTIFICATION_TYPE, type ActionDataParsed, type ActionInput } from '@/types';

const INITIAL: ActionInput = {
// @ts-ignore (TS complains about empty string because we expect an "ActionsType", but it's fine)
Expand All @@ -25,18 +25,31 @@ export function useActionFormData() {
switch (k) {
case 'type':
case 'signals':
if (Array.isArray(v) ? !v.length : !v) {
errors[k] = FORM_ALERTS.FIELD_IS_REQUIRED;
}
break;

case 'details':
if (Array.isArray(v) ? !v.length : !v) {
ok = false;
errors[k] = FORM_ALERTS.FIELD_IS_REQUIRED;
}
if (formData.type === ActionsType.LATENCY_SAMPLER) {
(safeJsonParse(v as string, { endpoints_filters: [] }) as LatencySamplerSpec).endpoints_filters.forEach((endpoint) => {
if (endpoint.http_route.charAt(0) !== '/') {
errors[k] = FORM_ALERTS.LATENCY_HTTP_ROUTE;
}
});
}
break;

default:
break;
}
});

ok = !Object.values(errors).length;

if (!ok && params?.withAlert) {
addNotification({
type: NOTIFICATION_TYPE.WARNING,
Expand Down
2 changes: 1 addition & 1 deletion frontend/webapp/hooks/sources/useSourceCRUD.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export const useSourceCRUD = (params?: Params) => {
} else {
const id = { kind, name, namespace };
if (!selected) removeNotifications(getSseTargetFromId(id, OVERVIEW_ENTITY_TYPES.SOURCE));
if (!selected) setConfiguredSources({ ...configuredSources, [namespace]: configuredSources[namespace].filter((source) => source.name !== name) });
if (!selected) setConfiguredSources({ ...configuredSources, [namespace]: configuredSources[namespace]?.filter((source) => source.name !== name) || [] });
handleComplete(action, `source "${name}" was ${action.toLowerCase()}d ${fromOrIn} "${namespace}"`, selected ? id : undefined);
}
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ export const AutocompleteInput: FC<Props> = ({ placeholder = 'Type to search...'
};

const handleKeyDown = (e: KeyboardEvent<HTMLInputElement>) => {
e.stopPropagation();
if (!['Enter'].includes(e.key)) e.stopPropagation();

// Flatten the options to handle keyboard navigation - TODO: Refactor this
return;
Expand Down
15 changes: 1 addition & 14 deletions frontend/webapp/reuseable-components/input-list/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -103,10 +103,6 @@ const InputList: React.FC<InputListProps> = ({ initialValues = [], value, onChan
});
};

const handleKeyDown: KeyboardEventHandler<HTMLInputElement> = (e) => {
e.stopPropagation();
};

// Check if any input field is empty
const isAddButtonDisabled = rows.some((input) => input.trim() === '');
const isDelButtonDisabled = rows.length <= 1;
Expand All @@ -118,16 +114,7 @@ const InputList: React.FC<InputListProps> = ({ initialValues = [], value, onChan
<ListContainer>
{rows.map((val, idx) => (
<RowWrapper key={`input-list-${idx}`}>
<Input
value={val}
onChange={(e) => {
e.stopPropagation();
handleInputChange(e.target.value, idx);
}}
onKeyDown={handleKeyDown}
hasError={!!errorMessage}
autoFocus={!val && rows.length > 1 && idx === rows.length - 1}
/>
<Input value={val} onChange={(e) => handleInputChange(e.target.value, idx)} hasError={!!errorMessage} autoFocus={!val && rows.length > 1 && idx === rows.length - 1} />
<DeleteButton disabled={isDelButtonDisabled} onClick={() => handleDeleteInput(idx)}>
<TrashIcon />
</DeleteButton>
Expand Down
10 changes: 1 addition & 9 deletions frontend/webapp/reuseable-components/input-table/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,6 @@ export const InputTable: React.FC<Props> = ({ columns, initialValues = [], value
});
};

const handleKeyDown: KeyboardEventHandler<HTMLInputElement> = (e) => {
e.stopPropagation();
};

// Check if any key or value field is empty
const isAddButtonDisabled = rows.some((row) => !!Object.values(row).filter((val) => !val).length);
const isDelButtonDisabled = rows.length <= 1;
Expand Down Expand Up @@ -143,11 +139,7 @@ export const InputTable: React.FC<Props> = ({ columns, initialValues = [], value
type={type}
placeholder={placeholder}
value={value}
onChange={({ stopPropagation, target: { value: val } }) => {
stopPropagation();
handleChange(keyName, type === 'number' ? Number(val) : val, idx);
}}
onKeyDown={handleKeyDown}
onChange={({ target: { value: val } }) => handleChange(keyName, type === 'number' ? Number(val) : val, idx)}
autoFocus={!value && rows.length > 1 && idx === rows.length - 1 && innerIdx === 0}
style={{ maxWidth, paddingLeft: 10 }}
hasError={!!errorMessage && (!required || (required && !value))}
Expand Down
105 changes: 48 additions & 57 deletions frontend/webapp/reuseable-components/input/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -115,61 +115,52 @@ const Button = styled.button`
`;

// Wrap Input with forwardRef to handle the ref prop
const Input = forwardRef<HTMLInputElement, InputProps>(
({ icon: Icon, buttonLabel, onButtonClick, hasError, errorMessage, title, tooltip, required, initialValue, value: v, onChange, type = 'text', name, ...props }, ref) => {
const isSecret = type === 'password';
const [revealSecret, setRevealSecret] = useState(false);
const [value, setValue] = useState<string>(v?.toString() || initialValue || '');

const handleInputChange = (e: ChangeEvent<HTMLInputElement>) => {
e.stopPropagation();
setValue(e.target.value);
onChange?.(e);
};

const handleKeyDown: KeyboardEventHandler<HTMLInputElement> = (e) => {
e.stopPropagation();
};

return (
<Container>
<FieldLabel title={title} required={required} tooltip={tooltip} />

<InputWrapper $disabled={props.disabled} $hasError={hasError || !!errorMessage} $isActive={!!props.autoFocus}>
{isSecret ? (
<IconWrapperClickable onClick={() => setRevealSecret((prev) => !prev)}>
{revealSecret ? <EyeClosedIcon size={14} fill={theme.text.grey} /> : <EyeOpenIcon size={14} fill={theme.text.grey} />}
</IconWrapperClickable>
) : Icon ? (
<IconWrapper>
<Icon size={14} fill={theme.text.grey} />
</IconWrapper>
) : null}

<StyledInput
ref={ref}
data-id={name}
type={revealSecret ? 'text' : type}
$hasIcon={!!Icon || isSecret}
name={name}
value={value}
onChange={handleInputChange}
onKeyDown={handleKeyDown}
{...props}
/>

{buttonLabel && onButtonClick && (
<Button onClick={onButtonClick} disabled={props.disabled}>
{buttonLabel}
</Button>
)}
</InputWrapper>

{!!errorMessage && <FieldError>{errorMessage}</FieldError>}
</Container>
);
},
);

Input.displayName = 'Input'; // Set a display name for easier debugging
const Input = forwardRef<HTMLInputElement, InputProps>(({ icon: Icon, buttonLabel, onButtonClick, hasError, errorMessage, title, tooltip, required, onChange, type = 'text', name, ...props }, ref) => {
const isSecret = type === 'password';
const [revealSecret, setRevealSecret] = useState(false);

const handleInputChange = (e: ChangeEvent<HTMLInputElement>) => {
e.stopPropagation();

const v = e.target.value;
const actualValue = type === 'number' ? v.replace(/[^\d]/g, '') : v;
e.target.value = actualValue;

onChange?.(e);
};

const handleKeyDown: KeyboardEventHandler<HTMLInputElement> = (e) => {
if (!['Enter'].includes(e.key)) e.stopPropagation();
};

return (
<Container>
<FieldLabel title={title} required={required} tooltip={tooltip} />

<InputWrapper $disabled={props.disabled} $hasError={hasError || !!errorMessage} $isActive={!!props.autoFocus}>
{isSecret ? (
<IconWrapperClickable onClick={() => setRevealSecret((prev) => !prev)}>
{revealSecret ? <EyeClosedIcon size={14} fill={theme.text.grey} /> : <EyeOpenIcon size={14} fill={theme.text.grey} />}
</IconWrapperClickable>
) : Icon ? (
<IconWrapper>
<Icon size={14} fill={theme.text.grey} />
</IconWrapper>
) : null}

<StyledInput ref={ref} data-id={name} type={revealSecret ? 'text' : type} $hasIcon={!!Icon || isSecret} name={name} onChange={handleInputChange} onKeyDown={handleKeyDown} {...props} />

{buttonLabel && onButtonClick && (
<Button onClick={onButtonClick} disabled={props.disabled}>
{buttonLabel}
</Button>
)}
</InputWrapper>

{!!errorMessage && <FieldError>{errorMessage}</FieldError>}
</Container>
);
});

Input.displayName = 'Input';
export { Input };
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@ export const KeyValueInputsList: React.FC<KeyValueInputsListProps> = ({ initialK
});
};

const handleKeyDown: KeyboardEventHandler<HTMLInputElement> = (e) => {
e.stopPropagation();
};

// Check if any key or value field is empty
const isAddButtonDisabled = rows.some(({ key, value }) => key.trim() === '' || value.trim() === '');
const isDelButtonDisabled = rows.length <= 1;
Expand All @@ -129,11 +125,7 @@ export const KeyValueInputsList: React.FC<KeyValueInputsListProps> = ({ initialK
<Input
placeholder='Attribute name'
value={key}
onChange={(e) => {
e.stopPropagation();
handleChange('key', e.target.value, idx);
}}
onKeyDown={handleKeyDown}
onChange={(e) => handleChange('key', e.target.value, idx)}
hasError={!!errorMessage && (!required || (required && !key))}
autoFocus={!value && rows.length > 1 && idx === rows.length - 1}
/>
Expand All @@ -143,11 +135,7 @@ export const KeyValueInputsList: React.FC<KeyValueInputsListProps> = ({ initialK
<Input
placeholder='Attribute value'
value={value}
onChange={(e) => {
e.stopPropagation();
handleChange('value', e.target.value, idx);
}}
onKeyDown={handleKeyDown}
onChange={(e) => handleChange('value', e.target.value, idx)}
hasError={!!errorMessage && (!required || (required && !value))}
autoFocus={false}
/>
Expand Down
Loading

0 comments on commit 1cd219d

Please sign in to comment.