Skip to content

Commit

Permalink
[GEN-2080]: fix useConnectDestinationForm to support additional compo…
Browse files Browse the repository at this point in the history
…nent properties for checkboxes (#2043)

This pull request focuses on replacing the `initialValue` prop with the
`value` prop for the `Checkbox` component across various parts of the
codebase. This change aims to standardize the usage of the `Checkbox`
component and ensure consistency in its implementation.

Key changes include:

*Checkbox Component Update:*
*
[`frontend/webapp/reuseable-components/checkbox/index.tsx`](diffhunk://#diff-e3e26dd8177396b9bc7c42ef92c2501572b33fed1806496d512ed90c2539f831L12-R12):
Replaced `initialValue` prop with `value` prop in the `Checkbox`
component. Updated the state management and useEffect hook accordingly.
[[1]](diffhunk://#diff-e3e26dd8177396b9bc7c42ef92c2501572b33fed1806496d512ed90c2539f831L12-R12)
[[2]](diffhunk://#diff-e3e26dd8177396b9bc7c42ef92c2501572b33fed1806496d512ed90c2539f831L39-R41)

*Usage in Forms and Fields:*
*
[`frontend/webapp/containers/main/actions/action-form-body/custom-fields/pii-masking.tsx`](diffhunk://#diff-1d91fa50761c80281ff07b7acecbeceb87478bdfd17634c6ee9ff0dd8905b5d5L69-R69):
Updated `Checkbox` component to use `value` instead of `initialValue`.
*
[`frontend/webapp/containers/main/destinations/destination-form-body/dynamic-fields/index.tsx`](diffhunk://#diff-c17889e1ffea148a206f57df5da9f8cc755e7e522a9057a339f6887c01ed5815L27-R27):
Updated `Checkbox` component to use `value` instead of `initialValue`.
*
[`frontend/webapp/containers/main/instrumentation-rules/rule-form-body/custom-fields/payload-collection.tsx`](diffhunk://#diff-e24d6b5b41b8be540f0fb09e0cc33f0d4aff8a7e61e19053129e9e1d36c845daL94-R94):
Updated `Checkbox` component to use `value` instead of `initialValue`.

*Usage in Source Selection:*
*
[`frontend/webapp/containers/main/sources/choose-sources/choose-sources-body/choose-sources-body-fast/source-controls/index.tsx`](diffhunk://#diff-9730809c6d075f90b2e6e31dc914c93f5507fc1ad62343607b690162c0af150dL37-R37):
Updated `Checkbox` component to use `value` instead of `initialValue`.
*
[`frontend/webapp/containers/main/sources/choose-sources/choose-sources-body/choose-sources-body-fast/sources-list/index.tsx`](diffhunk://#diff-6b67649d370d208941f4e5a78c0c9de2f5b9b65fd49e0ede8ef57066982450aaL142-R142):
Updated `Checkbox` component to use `value` instead of `initialValue`.
[[1]](diffhunk://#diff-6b67649d370d208941f4e5a78c0c9de2f5b9b65fd49e0ede8ef57066982450aaL142-R142)
[[2]](diffhunk://#diff-6b67649d370d208941f4e5a78c0c9de2f5b9b65fd49e0ede8ef57066982450aaL169-R169)

*Other Updates:*
*
[`frontend/webapp/hooks/destinations/useConnectDestinationForm.ts`](diffhunk://#diff-ff55b24fb020911d3ee70fd88fce706bdc4348bfdebe633f7e6967f560218b4eL8-R61):
Refactored the `buildFormDynamicFields` function to use the `value` prop
for various input types including `CHECKBOX`.
*
[`frontend/webapp/reuseable-components/dropdown/index.tsx`](diffhunk://#diff-707b6e11fbf977d74c45816fab07924a0eaa2d139b296cc58b74c98fd76f463dL253-R253):
Updated `Checkbox` component to use `value` instead of `initialValue` in
the dropdown list item.

These changes ensure that the `Checkbox` component is consistently using
the `value` prop, which improves the readability and maintainability of
the codebase.
  • Loading branch information
BenElferink authored Dec 23, 2024
1 parent 5f3b442 commit 21b705d
Show file tree
Hide file tree
Showing 14 changed files with 48 additions and 59 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const PiiMasking: React.FC<Props> = ({ value, setValue, errorMessage }) => {
<FieldLabel title='Attributes to mask' required />
<ListContainer $hasError={!!errorMessage}>
{strictPicklist.map(({ id, label }) => (
<Checkbox key={id} title={label} disabled={isLastSelection && mappedValue.includes(id)} initialValue={mappedValue.includes(id)} onChange={(bool) => handleChange(id, bool)} />
<Checkbox key={id} title={label} disabled={isLastSelection && mappedValue.includes(id)} value={mappedValue.includes(id)} onChange={(bool) => handleChange(id, bool)} />
))}
</ListContainer>
{!!errorMessage && <FieldError>{errorMessage}</FieldError>}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const DestinationDynamicFields: React.FC<Props> = ({ fields, onChange, fo
case INPUT_TYPES.TEXTAREA:
return <TextArea key={field.name} {...rest} onChange={(e) => onChange(field.name, e.target.value)} errorMessage={formErrors[field.name]} />;
case INPUT_TYPES.CHECKBOX:
return <Checkbox key={field.name} {...rest} initialValue={rest.value == 'true'} onChange={(bool) => onChange(field.name, String(bool))} errorMessage={formErrors[field.name]} />;
return <Checkbox key={field.name} {...rest} value={rest.value == 'true'} onChange={(bool) => onChange(field.name, String(bool))} errorMessage={formErrors[field.name]} />;
default:
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ const PayloadCollection: React.FC<Props> = ({ value, setValue, formErrors }) =>
<FieldLabel title='Type of data to collect' required />
<ListContainer $hasError={!!errorMessage}>
{strictPicklist.map(({ id, label }) => (
<Checkbox key={id} title={label} disabled={isLastSelection && mappedValue.includes(id)} initialValue={mappedValue.includes(id)} onChange={(bool) => handleChange(id, bool)} />
<Checkbox key={id} title={label} disabled={isLastSelection && mappedValue.includes(id)} value={mappedValue.includes(id)} onChange={(bool) => handleChange(id, bool)} />
))}
</ListContainer>
{!!errorMessage && <FieldError>{errorMessage}</FieldError>}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export const SourceControls: React.FC<Props> = ({ selectedSources, searchText, s
<SearchWrapper>
<Input placeholder='Search Kubernetes Namespaces' icon={SearchIcon} value={searchText} onChange={(e) => setSearchText(e.target.value.toLowerCase())} />
</SearchWrapper>
{/* <Checkbox title='Select all' initialValue={selectAll} onChange={onSelectAll} /> */}
{/* <Checkbox title='Select all' value={selectAll} onChange={onSelectAll} /> */}
<Toggle title='Show selected only' initialValue={showSelectedOnly} onChange={setShowSelectedOnly} />
</FlexContainer>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ export const SourcesList: React.FC<Props> = ({
<Group data-id={`namespace-${namespace}`} key={`namespace-${namespace}`} $selected={isNamespaceAllSourcesSelected} $isOpen={isNamespaceSelected && hasFilteredSources}>
<NamespaceItem $selected={isNamespaceAllSourcesSelected} onClick={() => onSelectNamespace(namespace)}>
<FlexRow>
<Checkbox disabled={namespaceLoaded && !isNamespaceCanSelect} initialValue={isNamespaceAllSourcesSelected} onChange={(bool) => onSelectAll(bool, namespace)} />
<Checkbox disabled={namespaceLoaded && !isNamespaceCanSelect} value={isNamespaceAllSourcesSelected} onChange={(bool) => onSelectAll(bool, namespace)} />
<Text>{namespace}</Text>
</FlexRow>

Expand All @@ -166,7 +166,7 @@ export const SourcesList: React.FC<Props> = ({
return (
<SourceItem key={`source-${source.name}`} $selected={isSourceSelected} onClick={() => onSelectSource(source)}>
<FlexRow>
<Checkbox initialValue={isSourceSelected} onChange={() => onSelectSource(source, namespace)} />
<Checkbox value={isSourceSelected} onChange={() => onSelectSource(source, namespace)} />
<Text>{source.name}</Text>
<Text opacity={0.8} size={10}>
{source.numberOfInstances} running instance{source.numberOfInstances !== 1 && 's'} · {source.kind}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,7 @@ export const SourceControls: React.FC<Props> = ({
<ToggleWrapper>
<Toggle title='Select all' initialValue={selectAll} onChange={onSelectAll} />
<Toggle title='Show selected only' initialValue={showSelectedOnly} onChange={setShowSelectedOnly} />
<Checkbox
title='Future apps'
tooltip='Automatically instrument all future apps'
initialValue={!!selectedNamespace ? selectedFutureApps[selectedNamespace] : false}
onChange={onSelectFutureApps}
/>
<Checkbox title='Future apps' tooltip='Automatically instrument all future apps' value={!!selectedNamespace ? selectedFutureApps[selectedNamespace] : false} onChange={onSelectFutureApps} />
</ToggleWrapper>
</FlexContainer>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const SourcesList: React.FC<Props> = ({

{isSelected && (
<SelectedTextWrapper>
<Checkbox initialValue={true} />
<Checkbox value={true} />
</SelectedTextWrapper>
)}
</ListItem>
Expand Down
60 changes: 28 additions & 32 deletions frontend/webapp/hooks/destinations/useConnectDestinationForm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,64 +5,60 @@ export function useConnectDestinationForm() {
function buildFormDynamicFields(fields: DestinationDetailsField[]): DynamicField[] {
return fields
.map((field) => {
const { name, componentType, displayName, componentProperties, initialValue } = field;
const { componentType, displayName, initialValue, componentProperties, ...restOfField } = field;

let componentPropertiesJson;
let initialValuesJson;
switch (componentType) {
case INPUT_TYPES.DROPDOWN:
componentPropertiesJson = safeJsonParse<{ [key: string]: string }>(componentProperties, {});

const options = Array.isArray(componentPropertiesJson.values)
? componentPropertiesJson.values.map((value) => ({
id: value,
value,
}))
: Object.entries(componentPropertiesJson.values).map(([key, value]) => ({
id: key,
value,
}));

return {
name,
componentType,
title: displayName,
onSelect: () => {},
options,
placeholder: 'Select an option',
...componentPropertiesJson,
};

switch (componentType) {
case INPUT_TYPES.INPUT:
case INPUT_TYPES.TEXTAREA:
componentPropertiesJson = safeJsonParse<string[]>(componentProperties, []);
case INPUT_TYPES.CHECKBOX:
case INPUT_TYPES.KEY_VALUE_PAIR:
componentPropertiesJson = safeJsonParse<{ [key: string]: string }>(componentProperties, {});

return {
name,
componentType,
title: displayName,
value: initialValue,
...restOfField,
...componentPropertiesJson,
};

case INPUT_TYPES.MULTI_INPUT:
componentPropertiesJson = safeJsonParse<string[]>(componentProperties, []);
componentPropertiesJson = safeJsonParse<{ [key: string]: string }>(componentProperties, {});
initialValuesJson = safeJsonParse<string[]>(initialValue, []);

return {
name,
componentType,
title: displayName,
initialValues: initialValuesJson,
value: initialValuesJson,
initialValues: initialValuesJson,
...restOfField,
...componentPropertiesJson,
};

case INPUT_TYPES.KEY_VALUE_PAIR:
case INPUT_TYPES.CHECKBOX:
case INPUT_TYPES.DROPDOWN:
componentPropertiesJson = safeJsonParse<{ [key: string]: string }>(componentProperties, {});

const options = Array.isArray(componentPropertiesJson.values)
? componentPropertiesJson.values.map((value) => ({
id: value,
value,
}))
: Object.entries(componentPropertiesJson.values).map(([key, value]) => ({
id: key,
value,
}));

return {
name,
componentType,
title: displayName,
value: initialValue,
placeholder: componentPropertiesJson.placeholder || 'Select an option',
options,
onSelect: () => {},
...restOfField,
...componentPropertiesJson,
};

Expand Down
8 changes: 4 additions & 4 deletions frontend/webapp/reuseable-components/checkbox/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ interface CheckboxProps {
title?: string;
titleColor?: React.CSSProperties['color'];
tooltip?: string;
initialValue?: boolean;
value?: boolean;
onChange?: (value: boolean) => void;
disabled?: boolean;
style?: React.CSSProperties;
Expand All @@ -36,9 +36,9 @@ const CheckboxWrapper = styled.div<{ $isChecked: boolean; $disabled?: CheckboxPr
transition: border 0.3s, background-color 0.3s;
`;

export const Checkbox: React.FC<CheckboxProps> = ({ title, titleColor, tooltip, initialValue = false, onChange, disabled, style }) => {
const [isChecked, setIsChecked] = useState(initialValue);
useEffect(() => setIsChecked(initialValue), [initialValue]);
export const Checkbox: React.FC<CheckboxProps> = ({ title, titleColor, tooltip, value = false, onChange, disabled, style }) => {
const [isChecked, setIsChecked] = useState(value);
useEffect(() => setIsChecked(value), [value]);

const handleToggle: React.MouseEventHandler<HTMLDivElement> = (e) => {
if (disabled) return;
Expand Down
2 changes: 1 addition & 1 deletion frontend/webapp/reuseable-components/dropdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ const DropdownListItem: React.FC<{
if (isMulti) {
return (
<DropdownItem className={isSelected ? 'selected' : ''}>
<Checkbox title={option.value} titleColor={theme.text.secondary} initialValue={isSelected} onChange={(toAdd) => (toAdd ? onSelect(option) : onDeselect?.(option))} style={{ width: '100%' }} />
<Checkbox title={option.value} titleColor={theme.text.secondary} value={isSelected} onChange={(toAdd) => (toAdd ? onSelect(option) : onDeselect?.(option))} style={{ width: '100%' }} />
</DropdownItem>
);
}
Expand Down
8 changes: 4 additions & 4 deletions frontend/webapp/reuseable-components/input/index.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import React, { useState, forwardRef, type ChangeEvent, type KeyboardEventHandler } from 'react';
import React, { useState, forwardRef, type ChangeEvent, type KeyboardEventHandler, type InputHTMLAttributes } from 'react';
import theme from '@/styles/theme';
import styled, { css } from 'styled-components';
import { EyeClosedIcon, EyeOpenIcon, SVG } from '@/assets';
import { FieldError, FieldLabel } from '@/reuseable-components';

interface InputProps extends React.InputHTMLAttributes<HTMLInputElement> {
interface InputProps extends InputHTMLAttributes<HTMLInputElement> {
title?: string;
icon?: SVG;
tooltip?: string;
Expand Down Expand Up @@ -116,10 +116,10 @@ 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, onChange, type = 'text', name, ...props }, ref) => {
({ 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>(initialValue || '');
const [value, setValue] = useState<string>(v?.toString() || initialValue || '');

const handleInputChange = (e: ChangeEvent<HTMLInputElement>) => {
e.stopPropagation();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,7 @@ export const MonitoringCheckboxes: React.FC<Props> = ({ isVertical, title = 'Mon

if (!allowed) return null;

return (
<Checkbox key={monitor.id} title={monitor.title} disabled={!allowed || (isLastSelection && selected)} initialValue={selected} onChange={(value) => handleChange(monitor.type, value)} />
);
return <Checkbox key={monitor.id} title={monitor.title} disabled={!allowed || (isLastSelection && selected)} value={selected} onChange={(value) => handleChange(monitor.type, value)} />;
})}
</ListContainer>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ const BaseNode: React.FC<Props> = ({ id: nodeId, data }) => {
) : // : type === 'source' && SOME_INDICATOR_THAT_THIS_IS_INSTRUMENTED ? ( <Image src={getEntityIcon(OVERVIEW_ENTITY_TYPES.RULE)} alt='' width={18} height={18} /> )
null}

{type === 'source' ? <Checkbox initialValue={getSourceLocation().index !== -1} onChange={onSelectSource} /> : null}
{type === 'source' ? <Checkbox value={getSourceLocation().index !== -1} onChange={onSelectSource} /> : null}
</>
);
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const HeaderNode: React.FC<Props> = ({ data }) => {

return (
<ActionsWrapper>
<Checkbox initialValue={sources.length === totalSelectedSources} onChange={onSelect} />
<Checkbox value={sources.length === totalSelectedSources} onChange={onSelect} />
</ActionsWrapper>
);
};
Expand Down

0 comments on commit 21b705d

Please sign in to comment.