Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export const ToggleBase = ({ className, isHovered, isDisabled, isFocusVisible, i
);
};

export interface ToggleProps extends AriaSwitchProps {
interface ToggleProps extends AriaSwitchProps {
size?: "sm" | "md";
label?: string;
hint?: ReactNode;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ const AddCustomProperty = ({
() => ({
name: 'multiSelect',
label: t('label.multi-select'),
type: FieldTypes.UT_SWITCH,
type: FieldTypes.SWITCH_MUI,
required: false,
props: {
'data-testid': 'multiSelect',
Expand Down Expand Up @@ -559,8 +559,7 @@ const AddCustomProperty = ({
layout="vertical"
onFieldsChange={handleFieldsChange}
onFinish={handleFormSubmit}
onFocus={handleFieldFocus}
>
onFocus={handleFieldFocus}>
{generateFormFields(formFields)}
{
// Only show enum value field if the property type has enum config
Expand All @@ -586,8 +585,7 @@ const AddCustomProperty = ({
<Button
data-testid="back-button"
type="link"
onClick={handleCancel}
>
onClick={handleCancel}>
{t('label.back')}
</Button>
</Col>
Expand All @@ -596,8 +594,7 @@ const AddCustomProperty = ({
data-testid="create-button"
htmlType="submit"
loading={isCreating || loading}
type="primary"
>
type="primary">
{t('label.create')}
</Button>
</Col>
Expand All @@ -624,8 +621,7 @@ const AddCustomProperty = ({
</div>
}
title={t('label.add-entity', { entity: t('label.custom-property') })}
onClose={onClose ?? handleCancel}
>
onClose={onClose ?? handleCancel}>
{formContent}
</MuiDrawer>
);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/*
* Copyright 2025 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { SwitchProps } from '@mui/material';

export interface MUISwitchProps extends Omit<SwitchProps, 'onChange'> {
checked?: boolean;
onChange?: (checked: boolean) => void;
label?: string;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
/*
* Copyright 2025 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { fireEvent, render, screen } from '@testing-library/react';
import MUISwitch from './MUISwitch';

describe('MUISwitch Component', () => {
it('should render the switch component', () => {
render(<MUISwitch />);

const switchElement = screen.getByRole('switch');

expect(switchElement).toBeInTheDocument();
});

it('should render with unchecked state by default', () => {
render(<MUISwitch />);

const switchElement = screen.getByRole('switch');

expect(switchElement).not.toBeChecked();
});

it('should render with checked state when checked prop is true', () => {
render(<MUISwitch checked />);

const switchElement = screen.getByRole('switch');

expect(switchElement).toBeChecked();
});

it('should render label when provided', () => {
const labelText = 'Enable Feature';

render(<MUISwitch label={labelText} />);

expect(screen.getByText(labelText)).toBeInTheDocument();
});

it('should not render label when not provided', () => {
render(<MUISwitch />);

expect(screen.queryByText('Enable Feature')).not.toBeInTheDocument();
});

it('should call onChange with true when switch is toggled on', () => {
const mockOnChange = jest.fn();

render(<MUISwitch checked={false} onChange={mockOnChange} />);

const switchElement = screen.getByRole('switch');

fireEvent.click(switchElement);

expect(mockOnChange).toHaveBeenCalledTimes(1);
expect(mockOnChange).toHaveBeenCalledWith(true);
});

it('should call onChange with false when switch is toggled off', () => {
const mockOnChange = jest.fn();

render(<MUISwitch checked onChange={mockOnChange} />);

const switchElement = screen.getByRole('switch');

fireEvent.click(switchElement);

expect(mockOnChange).toHaveBeenCalledTimes(1);
expect(mockOnChange).toHaveBeenCalledWith(false);
});

it('should not throw error when onChange is not provided', () => {
render(<MUISwitch />);

const switchElement = screen.getByRole('switch');

expect(() => fireEvent.click(switchElement)).not.toThrow();
});

it('should apply disabled attribute when disabled prop is true', () => {
render(<MUISwitch disabled />);

const switchElement = screen.getByRole('switch');

expect(switchElement).toBeDisabled();
});

it('should not call onChange when disabled', () => {
const mockOnChange = jest.fn();

render(<MUISwitch disabled onChange={mockOnChange} />);

const switchElement = screen.getByRole('switch');

fireEvent.click(switchElement);

expect(mockOnChange).not.toHaveBeenCalled();
});

it('should pass additional props to the underlying Switch component', () => {
render(<MUISwitch data-testid="custom-switch" size="small" />);

expect(screen.getByTestId('custom-switch')).toBeInTheDocument();
});

it('should render with both label and checked state', () => {
const labelText = 'Toggle Me';

render(<MUISwitch checked label={labelText} />);

const switchElement = screen.getByRole('switch');

expect(switchElement).toBeChecked();

expect(screen.getByText(labelText)).toBeInTheDocument();
});

it('should support memoization without breaking rendering', () => {
const { rerender } = render(<MUISwitch />);

rerender(<MUISwitch />);

expect(screen.getByRole('switch')).toBeInTheDocument();
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/*
* Copyright 2025 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
import { Box, Switch as MuiSwitch, SxProps, Theme } from '@mui/material';
import { FC, memo, useCallback } from 'react';
import MUIFormItemLabel from '../../common/MUIFormItemLabel/MUIFormItemLabel';
import { MUISwitchProps } from './MUISwitch.interface';

const LABEL_STYLES: SxProps<Theme> = {
color: (theme) => theme.palette.grey[700],
fontWeight: (theme) => theme.typography.subtitle2.fontWeight,
};

const MUISwitch: FC<MUISwitchProps> = ({
checked = false,
onChange,
label,
disabled = false,
...props
}) => {
const handleChange = useCallback(
(event: React.ChangeEvent<HTMLInputElement>) => {
if (!disabled) {
onChange?.(event.target.checked);
}
},
[onChange, disabled]
);

return (
<Box sx={{ display: 'flex', gap: 2, alignItems: 'center' }}>
<MuiSwitch
checked={checked}
disabled={disabled}
onChange={handleChange}
{...props}
/>
{label && <MUIFormItemLabel label={label} labelSx={LABEL_STYLES} />}
</Box>
);
};

export default memo(MUISwitch);
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/*
* Copyright 2025 Collate.
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
* http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
export { default } from './MUISwitch';
export type { MUISwitchProps } from './MUISwitch.interface';
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export enum FieldTypes {
PASSWORD_MUI = 'password_mui',
FILTER_PATTERN = 'filter_pattern',
SWITCH = 'switch',
UT_SWITCH = 'ut_switch',
SWITCH_MUI = 'switch_mui',
SELECT = 'select',
SELECT_MUI = 'select_mui',
ASYNC_SELECT_LIST = 'async_select_list',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,28 +46,6 @@ jest.mock('@openmetadata/ui-core-components', () => {
children: React.ReactNode;
className?: string;
}) => <button className={className}>{children}</button>,
Toggle: ({
id,
isSelected,
onChange,
isDisabled,
className,
}: {
id?: string;
isSelected?: boolean;
onChange?: (val: boolean) => void;
isDisabled?: boolean;
className?: string;
}) => (
<button
aria-checked={isSelected}
className={className}
disabled={isDisabled}
id={id}
role="switch"
onClick={() => onChange?.(!isSelected)}
/>
),
Grid: GridComponent,
};
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ const TagsForm = ({

return {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Disabled field's muiLabel is not translated in TagsForm

In TagsForm.tsx at line 215-218, disabledField is spread and only label is translated (label: t(disabledField.label)), but muiLabel remains as the raw i18n key string 'label.disable-tag'. Since the field type is SWITCH_MUI, getField in formUtils.tsx uses muiLabel (not label) to render the switch label. This means the disabled toggle will display the literal string 'label.disable-tag' instead of the translated text.

Similarly, mutuallyExclusiveField (line 238-241) translates muiLabel but leaves label as the raw key — less impactful since SWITCH_MUI uses muiLabel, but still inconsistent.

Suggested fix:

fields.push({
  ...disabledField,
  label: t(disabledField.label),
  muiLabel: t(disabledField.muiLabel),
});

// And for mutuallyExclusiveField:
return {
  ...field,
  label: t(field.label),
  muiLabel: t(field.muiLabel),
};

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

...field,
label: t(field.label),
muiLabel: t(field.muiLabel),
};
}, [t, disableMutuallyExclusiveField, isMutuallyExclusive]);

Expand Down Expand Up @@ -278,8 +278,9 @@ const TagsForm = ({
return (
<EntityAttachmentProvider
entityFqn={initialValues?.fullyQualifiedName}
entityType={isClassification ? EntityType.CLASSIFICATION : EntityType.TAG}
>
entityType={
isClassification ? EntityType.CLASSIFICATION : EntityType.TAG
}>
<Form
className="tags-form"
data-testid="tags-form"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,13 +356,16 @@ describe('tagFormFields', () => {
name: 'disabled',
required: false,
label: 'label.disable-tag',
muiLabel: 'label.disable-tag',
id: 'root/disabled',
type: FieldTypes.UT_SWITCH,
type: FieldTypes.SWITCH_MUI,
formItemLayout: FormItemLayout.HORIZONTAL,
props: {
'data-testid': 'disabled',
inputProps: {
'data-testid': 'disabled',
},
initialValue: false,
isDisabled: false,
disabled: false,
},
});
});
Expand All @@ -382,7 +385,7 @@ describe('tagFormFields', () => {
disabled: true,
});

expect(result.props?.isDisabled).toBe(true);
expect(result.props?.disabled).toBe(true);
});

it('should have horizontal form item layout', () => {
Expand All @@ -405,12 +408,13 @@ describe('tagFormFields', () => {
expect(result).toEqual({
name: 'mutuallyExclusive',
label: 'label.mutually-exclusive',
type: FieldTypes.UT_SWITCH,
muiLabel: 'label.mutually-exclusive',
type: FieldTypes.SWITCH_MUI,
required: false,
props: {
id: 'tags_mutuallyExclusive',
'data-testid': 'mutually-exclusive-button',
isDisabled: false,
disabled: false,
className: 'mutually-exclusive-switch',
},
helperTextType: HelperTextType.ALERT,
Expand All @@ -434,7 +438,7 @@ describe('tagFormFields', () => {
showHelperText: false,
});

expect(result.props?.isDisabled).toBe(true);
expect(result.props?.disabled).toBe(true);
});

it('should have ALERT helper text type', () => {
Expand Down
Loading
Loading