Render depends on menu items conditionally in chart settings + fix min max values (#15391)

closes
https://discord.com/channels/1130383047699738754/1430894689317294152

---------

Co-authored-by: Lucas Bordeau <bordeau.lucas@gmail.com>
Co-authored-by: Raphaël Bosi <71827178+bosiraphael@users.noreply.github.com>
This commit is contained in:
nitin
2025-10-30 20:56:37 +05:30
committed by GitHub
co-authored by Lucas Bordeau Raphaël Bosi
parent 2510cbf614
commit ab0cb67686
8 changed files with 185 additions and 77 deletions
@@ -49,8 +49,8 @@ export const CommandMenuItemNumberInput = ({
const numericValue = castAsNumberOrNull(draftValue);
if (isDefined(onValidate)) {
const isInvalid = onValidate(numericValue);
if (isInvalid) {
const isValid = onValidate(numericValue);
if (!isValid) {
setHasError(true);
return;
}
@@ -86,6 +86,7 @@ export const CommandMenuItemNumberInput = ({
onBlur={handleBlur}
onKeyDown={handleKeyDown}
placeholder={placeholder}
error={hasError ? ' ' : undefined}
noErrorHelper
/>
);
@@ -14,7 +14,7 @@ import { useUpdateChartSettingToggle } from '@/command-menu/pages/page-layout/ho
import { useUpdateCurrentWidgetConfig } from '@/command-menu/pages/page-layout/hooks/useUpdateCurrentWidgetConfig';
import { type ChartConfiguration } from '@/command-menu/pages/page-layout/types/ChartConfiguration';
import { CHART_CONFIGURATION_SETTING_IDS } from '@/command-menu/pages/page-layout/types/ChartConfigurationSettingIds';
import { isChartSettingDisabled } from '@/command-menu/pages/page-layout/utils/isChartSettingDisabled';
import { shouldHideChartSetting } from '@/command-menu/pages/page-layout/utils/shouldHideChartSetting';
import { CommandMenuPages } from '@/command-menu/types/CommandMenuPages';
import { useObjectMetadataItems } from '@/object-metadata/hooks/useObjectMetadataItems';
import { GRAPH_MAXIMUM_NUMBER_OF_GROUPS } from '@/page-layout/widgets/graph/constants/GraphMaximumNumberOfGroups.constant';
@@ -123,13 +123,21 @@ export const ChartSettings = ({ widget }: { widget: PageLayoutWidget }) => {
const chartSettings = GRAPH_TYPE_INFORMATION[currentGraphType].settings;
const visibleItemIds = chartSettings.flatMap((group) =>
group.items
.filter(
(item) =>
!shouldHideChartSetting(
item,
widget.objectMetadataId,
isGroupByEnabled as boolean,
),
)
.map((item) => item.id),
);
return (
<CommandMenuList
commandGroups={[]}
selectableItemIds={[
...chartSettings.flatMap((group) => group.items.map((item) => item.id)),
]}
>
<CommandMenuList commandGroups={[]} selectableItemIds={visibleItemIds}>
<ChartTypeSelectionSection
currentGraphType={currentGraphType}
setCurrentGraphType={handleGraphTypeChange}
@@ -139,52 +147,56 @@ export const ChartSettings = ({ widget }: { widget: PageLayoutWidget }) => {
message={t`Max ${GRAPH_MAXIMUM_NUMBER_OF_GROUPS} bars per chart. Consider adding a filter`}
/>
)}
{chartSettings.map((group) => (
<CommandGroup key={group.heading} heading={group.heading}>
{group.items.map((item) => {
const isDisabled = isChartSettingDisabled(
{chartSettings.map((group) => {
const visibleItems = group.items.filter(
(item) =>
!shouldHideChartSetting(
item,
widget.objectMetadataId,
isGroupByEnabled as boolean,
);
),
);
const handleItemToggleChange = () => {
setSelectedItemId(item.id);
updateChartSettingToggle(item.id);
};
return (
<CommandGroup key={group.heading} heading={group.heading}>
{visibleItems.map((item) => {
const handleItemToggleChange = () => {
setSelectedItemId(item.id);
updateChartSettingToggle(item.id);
};
const handleItemInputChange = (value: number | null) => {
updateChartSettingInput(item.id, value);
};
const handleItemInputChange = (value: number | null) => {
updateChartSettingInput(item.id, value);
};
const handleItemDropdownOpen = () => {
openDropdown({
dropdownComponentInstanceIdFromProps: item.id,
});
};
const handleItemDropdownOpen = () => {
openDropdown({
dropdownComponentInstanceIdFromProps: item.id,
});
};
const handleFilterClick = () => {
navigatePageLayoutCommandMenu({
commandMenuPage: CommandMenuPages.PageLayoutGraphFilter,
});
};
const handleFilterClick = () => {
navigatePageLayoutCommandMenu({
commandMenuPage: CommandMenuPages.PageLayoutGraphFilter,
});
};
return (
<ChartSettingItem
key={item.id}
item={item}
isDisabled={isDisabled}
configuration={configuration}
getChartSettingsValues={getChartSettingsValues}
onToggleChange={handleItemToggleChange}
onInputChange={handleItemInputChange}
onDropdownOpen={handleItemDropdownOpen}
onFilterClick={handleFilterClick}
/>
);
})}
</CommandGroup>
))}
return (
<ChartSettingItem
key={item.id}
item={item}
configuration={configuration}
getChartSettingsValues={getChartSettingsValues}
onToggleChange={handleItemToggleChange}
onInputChange={handleItemInputChange}
onDropdownOpen={handleItemDropdownOpen}
onFilterClick={handleFilterClick}
/>
);
})}
</CommandGroup>
);
})}
</CommandMenuList>
);
};
@@ -14,7 +14,6 @@ import { isDefined } from 'twenty-shared/utils';
type ChartSettingItemProps = {
item: ChartSettingsItem;
isDisabled: boolean;
configuration: ChartConfiguration;
getChartSettingsValues: (
itemId: CHART_CONFIGURATION_SETTING_IDS,
@@ -27,7 +26,6 @@ type ChartSettingItemProps = {
export const ChartSettingItem = ({
item,
isDisabled,
configuration,
getChartSettingsValues,
onToggleChange,
@@ -68,7 +66,7 @@ export const ChartSettingItem = ({
value={stringValue}
onChange={onInputChange}
onValidate={(value) =>
isDefined(value) &&
!isDefined(value) ||
isMinMaxRangeValid(
item.id as
| CHART_CONFIGURATION_SETTING_IDS.MIN_RANGE
@@ -92,7 +90,7 @@ export const ChartSettingItem = ({
<SelectableListItem
key={item.id}
itemId={item.id}
onEnter={isDisabled ? undefined : onToggleChange}
onEnter={onToggleChange}
>
<CommandMenuItemToggle
LeftIcon={item.Icon}
@@ -106,11 +104,7 @@ export const ChartSettingItem = ({
}
return (
<SelectableListItem
key={item.id}
itemId={item.id}
onEnter={isDisabled ? undefined : onDropdownOpen}
>
<SelectableListItem key={item.id} itemId={item.id} onEnter={onDropdownOpen}>
<CommandMenuItemDropdown
Icon={item.Icon}
label={t(item.label)}
@@ -125,7 +119,6 @@ export const ChartSettingItem = ({
description={getChartSettingsValues(item.id) as string}
contextualTextPosition={'right'}
hasSubMenu
disabled={isDisabled}
/>
</SelectableListItem>
);
@@ -114,4 +114,100 @@ describe('isMinMaxRangeValid', () => {
expect(result).toBe(true);
});
});
describe('setting first value (empty configuration)', () => {
it('should be valid when setting rangeMin on configuration without any range values', () => {
const emptyConfig = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MIN_RANGE,
50,
emptyConfig,
);
expect(result).toBe(true);
});
it('should be valid when setting rangeMax on configuration without any range values', () => {
const emptyConfig = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MAX_RANGE,
100,
emptyConfig,
);
expect(result).toBe(true);
});
it('should be valid when setting rangeMin without existing rangeMax', () => {
const configWithOnlyMin = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
rangeMin: 10,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MIN_RANGE,
20,
configWithOnlyMin,
);
expect(result).toBe(true);
});
it('should be valid when setting rangeMax without existing rangeMin', () => {
const configWithOnlyMax = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
rangeMax: 100,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MAX_RANGE,
150,
configWithOnlyMax,
);
expect(result).toBe(true);
});
it('should be invalid when setting rangeMin that would exceed existing rangeMax', () => {
const configWithOnlyMax = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
rangeMax: 100,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MIN_RANGE,
150,
configWithOnlyMax,
);
expect(result).toBe(false);
});
it('should be invalid when setting rangeMax that would be less than existing rangeMin', () => {
const configWithOnlyMin = {
__typename: 'BarChartConfiguration',
graphType: GraphType.VERTICAL_BAR,
rangeMin: 50,
} as ChartConfiguration;
const result = isMinMaxRangeValid(
CHART_CONFIGURATION_SETTING_IDS.MAX_RANGE,
10,
configWithOnlyMin,
);
expect(result).toBe(false);
});
});
});
@@ -2,9 +2,9 @@ import { CHART_CONFIGURATION_SETTING_IDS } from '@/command-menu/pages/page-layou
import { type ChartSettingsItem } from '@/command-menu/pages/page-layout/types/ChartSettingsGroup';
import { msg } from '@lingui/core/macro';
import { IconChartBar } from 'twenty-ui/display';
import { isChartSettingDisabled } from '../isChartSettingDisabled';
import { shouldHideChartSetting } from '../shouldHideChartSetting';
describe('isChartSettingDisabled', () => {
describe('shouldHideChartSetting', () => {
const mockItemWithoutDependencies: ChartSettingsItem = {
id: CHART_CONFIGURATION_SETTING_IDS.DATA_LABELS,
label: msg`Data Labels`,
@@ -45,7 +45,7 @@ describe('isChartSettingDisabled', () => {
describe('item without dependencies', () => {
it('should return false when item has no dependencies', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemWithoutDependencies,
'valid-object-id',
true,
@@ -55,7 +55,7 @@ describe('isChartSettingDisabled', () => {
});
it('should return false when item has no dependencies even with no object metadata', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemWithoutDependencies,
'',
false,
@@ -67,7 +67,7 @@ describe('isChartSettingDisabled', () => {
describe('item depending on SOURCE', () => {
it('should return true when no object metadata and item depends on SOURCE', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemDependingOnSource,
'',
true,
@@ -77,7 +77,7 @@ describe('isChartSettingDisabled', () => {
});
it('should return false when object metadata exists and item depends on SOURCE', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemDependingOnSource,
'valid-object-id',
true,
@@ -89,7 +89,7 @@ describe('isChartSettingDisabled', () => {
describe('item depending on GROUP_BY', () => {
it('should return true when group by is not enabled and item depends on GROUP_BY', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemDependingOnGroupBy,
'valid-object-id',
false,
@@ -99,7 +99,7 @@ describe('isChartSettingDisabled', () => {
});
it('should return false when group by is enabled and item depends on GROUP_BY', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemDependingOnGroupBy,
'valid-object-id',
true,
@@ -111,7 +111,7 @@ describe('isChartSettingDisabled', () => {
describe('item with multiple dependencies', () => {
it('should return true if any dependency is not met (no object metadata)', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemWithMultipleDependencies,
'',
true,
@@ -121,7 +121,7 @@ describe('isChartSettingDisabled', () => {
});
it('should return true if any dependency is not met (group by disabled)', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemWithMultipleDependencies,
'valid-object-id',
false,
@@ -131,7 +131,7 @@ describe('isChartSettingDisabled', () => {
});
it('should return false if all dependencies are met', () => {
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
mockItemWithMultipleDependencies,
'valid-object-id',
true,
@@ -152,7 +152,7 @@ describe('isChartSettingDisabled', () => {
dependsOn: undefined,
};
const result = isChartSettingDisabled(
const result = shouldHideChartSetting(
itemWithUndefinedDependsOn,
'',
false,
@@ -171,7 +171,7 @@ describe('isChartSettingDisabled', () => {
dependsOn: [],
};
const result = isChartSettingDisabled(itemWithEmptyDependsOn, '', false);
const result = shouldHideChartSetting(itemWithEmptyDependsOn, '', false);
expect(result).toBe(false);
});
@@ -9,12 +9,9 @@ export const isMinMaxRangeValid = (
newValue: number,
configuration: ChartConfiguration,
): boolean => {
if (!('rangeMax' in configuration || 'rangeMin' in configuration)) {
return false;
}
if (settingId === CHART_CONFIGURATION_SETTING_IDS.MIN_RANGE) {
if (
'rangeMax' in configuration &&
isDefined(configuration.rangeMax) &&
newValue > configuration.rangeMax
) {
@@ -24,6 +21,7 @@ export const isMinMaxRangeValid = (
if (settingId === CHART_CONFIGURATION_SETTING_IDS.MAX_RANGE) {
if (
'rangeMin' in configuration &&
isDefined(configuration.rangeMin) &&
newValue < configuration.rangeMin
) {
@@ -2,7 +2,7 @@ import { CHART_CONFIGURATION_SETTING_IDS } from '@/command-menu/pages/page-layou
import { type ChartSettingsItem } from '@/command-menu/pages/page-layout/types/ChartSettingsGroup';
import { isNonEmptyString } from '@sniptt/guards';
export const isChartSettingDisabled = (
export const shouldHideChartSetting = (
item: ChartSettingsItem,
objectMetadataId: string,
isGroupByEnabled: boolean,
@@ -25,6 +25,7 @@ export type MenuItemToggleProps = {
className?: string;
onToggleChange?: (toggled: boolean) => void;
toggleSize?: ToggleSize;
disabled?: boolean;
};
export const MenuItemToggle = ({
@@ -36,22 +37,29 @@ export const MenuItemToggle = ({
className,
onToggleChange,
toggleSize,
disabled = false,
}: MenuItemToggleProps) => {
const instanceId = useId();
return (
<StyledMenuItemBase className={className} focused={focused}>
<StyledMenuItemBase
className={className}
focused={focused}
disabled={disabled}
>
<StyledToggleContainer htmlFor={instanceId}>
<MenuItemLeftContent
LeftIcon={LeftIcon}
text={text}
withIconContainer={withIconContainer}
disabled={disabled}
/>
<StyledMenuItemRightContent>
<Toggle
id={instanceId}
value={toggled}
onChange={onToggleChange}
onChange={disabled ? undefined : onToggleChange}
toggleSize={toggleSize}
disabled={disabled}
/>
</StyledMenuItemRightContent>
</StyledToggleContainer>