Skip to content

Commit

Permalink
fix: added validation for the decimal places input #2567
Browse files Browse the repository at this point in the history
  • Loading branch information
Kausar-HM authored and ssjagad committed Mar 29, 2024
1 parent 829496c commit 57bcb3d
Show file tree
Hide file tree
Showing 7 changed files with 131 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import React from 'react';
import { PropertiesSection } from '~/customization/propertiesSectionComponent';
import { DashboardWidget } from '~/types';
import { maybeWithDefault } from '~/util/maybe';
import { SettingsSection } from './section';
import { DecimalPlacesSection } from './section';
import { CommonChartProperties } from '~/customization/widgets/types';
import { nonNegative } from '~/util/number';
import { PropertyLens } from '~/customization/propertiesSection';
Expand All @@ -12,7 +12,7 @@ const isSettingsWidget = (
w: DashboardWidget
): w is DashboardWidget<CommonChartProperties> => 'queryConfig' in w.properties;

const RenderSettingsConfiguration = ({
const RenderDecimalPlacesConfiguration = ({
useProperty,
}: {
useProperty: PropertyLens<DashboardWidget<CommonChartProperties>>;
Expand All @@ -27,17 +27,17 @@ const RenderSettingsConfiguration = ({
);

return (
<SettingsSection
<DecimalPlacesSection
significantDigits={maybeWithDefault(undefined, significantDigits)}
updateSignificantDigits={updateSignificantDigits}
/>
);
};
export const SettingsConfiguration: React.FC = () => (
export const FormatDataConfiguration: React.FC = () => (
<PropertiesSection
isVisible={isSettingsWidget}
render={({ useProperty }) => (
<RenderSettingsConfiguration useProperty={useProperty} />
<RenderDecimalPlacesConfiguration useProperty={useProperty} />
)}
/>
);
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
.settings-property-label {
display: flex;
align-items: center;
align-items: baseline;
flex-wrap: wrap;
font-weight: bold;
}

.settings-property-label-control {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { MOCK_LINE_CHART_WIDGET } from '../../../../testing/mocks';
import { Provider } from 'react-redux';
import { configureDashboardStore } from '~/store';
import { getByLabelText, render } from '@testing-library/react';
import { getByLabelText, render, screen } from '@testing-library/react';
import React from 'react';
import type { DashboardState } from '~/store/state';
import { SettingsConfiguration } from './index';
import { FormatDataConfiguration } from './index';
import userEvent from '@testing-library/user-event';

const state: Partial<DashboardState> = {
dashboardConfiguration: {
Expand All @@ -15,7 +16,7 @@ const state: Partial<DashboardState> = {

const TestComponent = () => (
<Provider store={configureDashboardStore(state)}>
<SettingsConfiguration />
<FormatDataConfiguration />
</Provider>
);

Expand All @@ -26,5 +27,16 @@ it('renders', () => {

it('renders the decimal places input', async () => {
const elem = render(<TestComponent />).baseElement;
expect(getByLabelText(elem, 'decimal places')).toBeTruthy();
expect(getByLabelText(elem, 'Decimal places')).toBeTruthy();
});

it('shows error if decimal places input is greater than 100', async () => {
const user = userEvent.setup();
render(<TestComponent />);
const input = screen.getByLabelText('Decimal places');
await user.type(input, '123');
const errorText = screen.getByText(
'Decimal places must be between 0 and 100.'
);
expect(errorText).toBeTruthy();
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
import React, { useEffect } from 'react';

import {
Box,
ExpandableSection,
FormField,
Input,
SpaceBetween,
} from '@cloudscape-design/components';

import './section.css';

import { isNumeric } from '@iot-app-kit/core/dist/es/common/number';
import { Controller, useForm } from 'react-hook-form';
import { useSelectedWidgets } from '~/hooks/useSelectedWidgets';
import { spaceScaledS } from '@cloudscape-design/design-tokens';

export const DecimalPlacesSection = ({
significantDigits,
updateSignificantDigits,
}: {
significantDigits: number | undefined;
updateSignificantDigits: (newValue: number | undefined) => void;
}) => {
const { control, setValue, clearErrors } = useForm<{
decimalPlaces: string;
}>({
mode: 'onChange',
});

const selectedWidgets = useSelectedWidgets();
const selectedWidgetId = selectedWidgets[0]?.id;

useEffect(() => {
//controller is using mode: 'onChange', it's not revalidating when different widget is selected
//when user selects different widget, manually set the significantDigits and clear the error state
setValue('decimalPlaces', significantDigits?.toFixed() || '');
clearErrors();
}, [clearErrors, setValue, significantDigits, selectedWidgetId]);

const onSignificantDigitsChange = (value: string) => {
const newValue = isNumeric(value) ? parseInt(value) || 0 : undefined;
if (newValue === undefined || newValue <= 100) {
updateSignificantDigits(newValue);
}
};

return (
<ExpandableSection
className='accordian-header'
headerText='Format data'
defaultExpanded
variant='footer'
>
<Box padding='s'>
<SpaceBetween size='m' direction='vertical'>
<div
className='settings-property-label'
style={{ gap: spaceScaledS }}
>
<label htmlFor='decimal-places'>Decimal places</label>
<div className='settings-property-label-control'>
<Controller
control={control}
name='decimalPlaces'
rules={{
min: {
value: 0,
message: 'Decimal places must be between 0 and 100.',
},
max: {
value: 100,
message: 'Decimal places must be between 0 and 100.',
},
}}
render={({ field, fieldState }) => (
<FormField
label=''
errorText={fieldState.error?.message}
constraintText='Must be between 0 and 100.'
>
<Input
type='number'
inputMode='numeric'
data-testid='decimal-place-config'
controlId='decimal-places'
value={field.value}
onChange={({ detail: { value } }) => {
field.onChange(value);
onSignificantDigitsChange(value);
}}
/>
</FormField>
)}
/>
</div>
</div>
</SpaceBetween>
</Box>
</ExpandableSection>
);
};
Original file line number Diff line number Diff line change
Expand Up @@ -150,14 +150,14 @@ describe(`${PropertiesPanel.name}`, () => {

expect(screen.getByText('Style')).toBeVisible();
expect(screen.getByText('Axis')).toBeVisible();
expect(screen.getByText('Settings')).toBeVisible();
expect(screen.getByText('Format data')).toBeVisible();
});
it('should render the style section when switched between widgets', async () => {
await renderTestComponentAsync();

expect(screen.getByText('Style')).toBeVisible();
expect(screen.getByText('Axis')).toBeVisible();
expect(screen.getByText('Settings')).toBeVisible();
expect(screen.getByText('Format data')).toBeVisible();

const thresholdsTab = screen.getByTestId('thresholds');
expect(thresholdsTab).toBeVisible();
Expand All @@ -172,7 +172,7 @@ describe(`${PropertiesPanel.name}`, () => {
};

await renderTestComponentAsync(options);
expect(screen.getByText('Settings')).toBeVisible();
expect(screen.getByText('Format data')).toBeVisible();
});

it('should render an empty thresholds section', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';

import { AggregationsSettingsConfiguration } from '../aggregationSettings';
import { AxisSettingsConfiguration } from '../axisSettings';
import { SettingsConfiguration } from '../settings';
import { FormatDataConfiguration } from '../formatDataSettings';
import { TextSettingsConfiguration } from '../textSettings';
import { LineAndScatterStyleSettingsSection } from '../lineAndScatterStyleSettings/section';
import { WidgetTitle } from '../widgetTitle';
Expand All @@ -11,11 +11,11 @@ import { DisplaySettingsSection } from '../displaySettingsSection';
export const StylesSection = () => (
<div>
<WidgetTitle />
<AggregationsSettingsConfiguration />
<FormatDataConfiguration />
<LineAndScatterStyleSettingsSection />
<DisplaySettingsSection />
<AggregationsSettingsConfiguration />
<AxisSettingsConfiguration />
<SettingsConfiguration />
<TextSettingsConfiguration />
</div>
);

This file was deleted.

0 comments on commit 57bcb3d

Please sign in to comment.