Skip to content

Commit

Permalink
feat(Conditionbuilder): renaming both variants to Hierarchical and No…
Browse files Browse the repository at this point in the history
…n-Hierarchical (carbon-design-system#5847)

* feat(conditionbuilder): design review changes 2

* feat(conditionBuilder): test case fix for delete conditions

* fix(ConditionBuilder): incomplete state icon and text order

* chore(ConditionBuilder): add typescript for condition builder files

* chore(ConditionBuilder): add typescript for condition builder files2

* feat(Conditionbuilder): renaming both variants

* feat(Conditionbuilder): renaming both variants 2

* fix(condition builder): review comments
  • Loading branch information
amal-k-joy authored Aug 22, 2024
1 parent f701002 commit 791e2b3
Show file tree
Hide file tree
Showing 15 changed files with 108 additions and 77 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ $block-class: #{c4p-settings.$pkg-prefix}--condition-builder;
margin-top: $spacing-03;
margin-bottom: $spacing-03;
}
.#{$block-class}__tree
.#{$block-class}__Hierarchical
.#{$block-class}__actions-container
.#{$block-class}__condition-wrapper {
flex-direction: column;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ $colors: (
}
}

.#{$block-class}__tree {
.#{$block-class}__Hierarchical {
.#{$block-class}__condition-wrapper > :nth-child(n + 3) {
flex-basis: 100%;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Close } from '@carbon/react/icons';
import { ConditionBuilderItem } from '../ConditionBuilderItem/ConditionBuilderItem';
import PropTypes from 'prop-types';
import {
HIERARCHICAL_VARIANT,
NON_HIERARCHICAL_VARIANT,
operatorConfig,
statementConfig,
} from '../ConditionBuilderContext/DataConfigs';
Expand Down Expand Up @@ -177,7 +179,7 @@ const ConditionBlock = (props: ConditionBlockProps) => {
setShowDeletionPreview(false);
};
const manageActionButtons = (conditionIndex, conditions) => {
if (variant === 'tree') {
if (variant === HIERARCHICAL_VARIANT) {
return true;
}
return isLastCondition(conditionIndex, conditions);
Expand All @@ -191,7 +193,7 @@ const ConditionBlock = (props: ConditionBlockProps) => {
);
};
const getAriaAttributes = () => {
return variant == 'tree'
return variant == HIERARCHICAL_VARIANT
? {
'aria-level': aria.level,
'aria-posinset': aria.posinset,
Expand Down Expand Up @@ -224,11 +226,11 @@ const ConditionBlock = (props: ConditionBlockProps) => {
[`${blockClass}__condition__deletion-preview`]: showDeletionPreview,
},
{
[`${blockClass}__gap-bottom`]: variant == 'tree',
[`${blockClass}__gap-bottom`]: variant == HIERARCHICAL_VARIANT,
},
{
[`${blockClass}__gap ${blockClass}__gap-bottom`]:
variant == 'sentence',
variant == NON_HIERARCHICAL_VARIANT,
},
{
[`${blockClass}__condition--interacting`]: showAllActions,
Expand Down Expand Up @@ -356,7 +358,7 @@ const ConditionBlock = (props: ConditionBlockProps) => {
hideConditionSubGroupPreviewHandler={
hideConditionSubGroupPreviewHandler
}
enableSubGroup={variant == 'tree'}
enableSubGroup={variant == HIERARCHICAL_VARIANT}
showConditionPreviewHandler={showConditionPreviewHandler}
hideConditionPreviewHandler={hideConditionPreviewHandler}
className={`${blockClass}__gap ${blockClass}__gap-left`}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,14 @@ import mdx from './ConditionBuilder.mdx';
import styles from './_storybook-styles.scss?inline';
import { inputData, inputDataDynamicOptions } from './assets/sampleInput';
import {
sampleDataStructure_sentence,
sampleDataStructure_tree,
sampleDataStructure_nonHierarchical,
sampleDataStructure_Hierarchical,
} from './assets/SampleData';
import uuidv4 from '../../global/js/utils/uuidv4';
import {
NON_HIERARCHICAL_VARIANT,
HIERARCHICAL_VARIANT,
} from './ConditionBuilderContext/DataConfigs';
export default {
title: 'Experimental/Components/ConditionBuilder',
component: ConditionBuilder,
Expand Down Expand Up @@ -223,61 +227,62 @@ export const conditionBuilder = ConditionBuilderTemplate.bind({});
conditionBuilder.storyName = 'Condition Builder';
conditionBuilder.args = {
inputConfig: inputData,
variant: 'sentence',
variant: NON_HIERARCHICAL_VARIANT,
};

export const conditionBuilderDynamicOptions = ConditionBuilderTemplate.bind({});
conditionBuilderDynamicOptions.storyName = 'With dynamic options';
conditionBuilderDynamicOptions.args = {
inputConfig: inputDataDynamicOptions,
getOptions: getOptions,
variant: 'sentence',
variant: NON_HIERARCHICAL_VARIANT,
};

export const conditionBuilderWithInitialState = ConditionBuilderTemplate.bind(
{}
);
conditionBuilderWithInitialState.storyName = 'With initial state';
conditionBuilderWithInitialState.args = {
initialState: sampleDataStructure_sentence,
initialState: sampleDataStructure_nonHierarchical,
inputConfig: inputData,
variant: 'sentence',
variant: NON_HIERARCHICAL_VARIANT,
translateWithId: translateWithId,
};

export const conditionBuilderWithActions = ConditionBuilderTemplate.bind({});
conditionBuilderWithActions.storyName = 'With Actions';
conditionBuilderWithActions.args = {
inputConfig: inputData,
variant: 'sentence',
variant: NON_HIERARCHICAL_VARIANT,
actions: actions,
getActionsState: (actionState) => {
console.log('action state', actionState);
},
};

export const conditionBuilderTree = ConditionBuilderTemplate.bind({});
conditionBuilderTree.storyName = 'Condition Builder(Tree)';
conditionBuilderTree.args = {
export const conditionBuilderHierarchical = ConditionBuilderTemplate.bind({});
conditionBuilderHierarchical.storyName = 'Condition Builder (Hierarchical)';
conditionBuilderHierarchical.args = {
inputConfig: inputData,
variant: 'tree',
variant: HIERARCHICAL_VARIANT,
};
export const conditionBuilderWithInitialStateTree =
export const conditionBuilderWithInitialStateHierarchical =
ConditionBuilderTemplate.bind({});
conditionBuilderWithInitialStateTree.storyName = 'With initial state(Tree)';
conditionBuilderWithInitialStateTree.args = {
initialState: sampleDataStructure_tree,
conditionBuilderWithInitialStateHierarchical.storyName =
'With initial state (Hierarchical)';
conditionBuilderWithInitialStateHierarchical.args = {
initialState: sampleDataStructure_Hierarchical,
inputConfig: inputData,
variant: 'tree',
variant: HIERARCHICAL_VARIANT,
};

export const conditionBuilderWithActionsTree = ConditionBuilderTemplate.bind(
{}
);
conditionBuilderWithActionsTree.storyName = 'With Actions(Tree)';
conditionBuilderWithActionsTree.args = {
export const conditionBuilderWithActionsHierarchical =
ConditionBuilderTemplate.bind({});
conditionBuilderWithActionsHierarchical.storyName =
'With Actions (Hierarchical)';
conditionBuilderWithActionsHierarchical.args = {
inputConfig: inputData,
variant: 'tree',
variant: HIERARCHICAL_VARIANT,
actions: actions,
getActionsState: (actionState) => {},
};
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@ import userEvent from '@testing-library/user-event';

import { inputData, inputDataDynamicOptions } from './assets/sampleInput';
import {
sampleDataStructure_sentence,
sampleDataStructure_tree,
sampleDataStructure_nonHierarchical,
sampleDataStructure_Hierarchical,
} from './assets/SampleData';
import {
NON_HIERARCHICAL_VARIANT,
HIERARCHICAL_VARIANT,
} from './ConditionBuilderContext/DataConfigs';

const blockClass = `${pkg.prefix}--condition-builder`;
const componentName = ConditionBuilder.displayName;
Expand All @@ -33,7 +37,7 @@ const defaultProps = {
startConditionLabel: 'Add condition',
popOverSearchThreshold: 4,
getConditionState: () => {},
variant: 'sentence',
variant: NON_HIERARCHICAL_VARIANT,
};

const inputConfigOptionType = {
Expand Down Expand Up @@ -134,7 +138,7 @@ describe(componentName, () => {
);
});

//test cases for sentence variant
//test cases for Non-Hierarchical variant
it('should render the component with provided label to start condition builder', async () => {
const startConditionLabel = 'Add condition';
render(
Expand Down Expand Up @@ -373,7 +377,7 @@ describe(componentName, () => {
<ConditionBuilder
{...defaultProps}
inputConfig={inputData}
initialState={sampleDataStructure_sentence}
initialState={sampleDataStructure_nonHierarchical}
/>
);
//start builder
Expand Down Expand Up @@ -603,7 +607,7 @@ describe(componentName, () => {
<ConditionBuilder
{...defaultProps}
inputConfig={inputData}
initialState={sampleDataStructure_sentence}
initialState={sampleDataStructure_nonHierarchical}
translateWithId={translateWithId}
/>
);
Expand All @@ -613,12 +617,12 @@ describe(componentName, () => {
expect(screen.getByText('Condition Heading'));
});

//test cases for tree variant
it('render the tree variant with 3 conditions and 1 subgroup', async () => {
//test cases for Hierarchical variant
it('render the Hierarchical variant with 3 conditions and 1 subgroup', async () => {
render(
<ConditionBuilder
{...defaultProps}
variant={'tree'}
variant={HIERARCHICAL_VARIANT}
inputConfig={inputData}
/>
);
Expand Down Expand Up @@ -706,11 +710,11 @@ describe(componentName, () => {
expect(subGroups).toHaveLength(2);
});

it('render the tree variant with 2 groups', async () => {
it('render the Hierarchical variant with 2 groups', async () => {
render(
<ConditionBuilder
{...defaultProps}
variant={'tree'}
variant={HIERARCHICAL_VARIANT}
inputConfig={inputData}
/>
);
Expand Down Expand Up @@ -842,7 +846,7 @@ describe(componentName, () => {
<ConditionBuilder
{...defaultProps}
inputConfig={inputData}
initialState={sampleDataStructure_sentence}
initialState={sampleDataStructure_nonHierarchical}
/>
);

Expand All @@ -868,7 +872,7 @@ describe(componentName, () => {
expect(closeButtons[0]).toHaveFocus();
});

it('check the next/previous close button is focussed on remove condition for tree variant', async () => {
it('check the next/previous close button is focussed on remove condition for Hierarchical variant', async () => {
const sampleDataStructure = {
operator: 'or',
groups: [
Expand Down Expand Up @@ -935,7 +939,7 @@ describe(componentName, () => {
<ConditionBuilder
{...defaultProps}
inputConfig={inputData}
variant="tree"
variant={HIERARCHICAL_VARIANT}
initialState={sampleDataStructure}
/>
);
Expand Down Expand Up @@ -1204,7 +1208,7 @@ describe(componentName, () => {
render(
<ConditionBuilder
{...defaultProps}
variant={'tree'}
variant={HIERARCHICAL_VARIANT}
inputConfig={inputData}
initialState={sampleDataStructure}
/>
Expand Down Expand Up @@ -1253,7 +1257,7 @@ describe(componentName, () => {
});

// keyboard navigation tests
//for sentence variant
//for Non-Hierarchical variant
it('add and remove conditions using keyboard', async () => {
render(
<ConditionBuilder
Expand Down Expand Up @@ -1382,12 +1386,12 @@ describe(componentName, () => {
expect(screen.getByText('Add condition')).toHaveFocus();
});

//for tree variant
//for Hierarchical variant
it('add and remove conditions using keyboard', async () => {
render(
<ConditionBuilder
{...defaultProps}
variant={'tree'}
variant={HIERARCHICAL_VARIANT}
inputConfig={inputData}
/>
);
Expand Down Expand Up @@ -1515,9 +1519,9 @@ describe(componentName, () => {
render(
<ConditionBuilder
{...defaultProps}
variant={'tree'}
variant={HIERARCHICAL_VARIANT}
inputConfig={inputData}
initialState={sampleDataStructure_tree}
initialState={sampleDataStructure_Hierarchical}
/>
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,10 @@ import { getDevtoolsProps } from '../../global/js/utils/devtools';
import ConditionBuilderContent from './ConditionBuilderContent/ConditionBuilderContent';
import { ConditionBuilderProvider } from './ConditionBuilderContext/ConditionBuilderProvider';
import { pkg } from '../../settings';
import { blockClass } from './ConditionBuilderContext/DataConfigs';
import {
blockClass,
NON_HIERARCHICAL_VARIANT,
} from './ConditionBuilderContext/DataConfigs';
import { handleKeyDown } from './utils/handleKeyboardEvents';

import { ConditionBuilderProps } from './ConditionBuilder.types';
Expand Down Expand Up @@ -61,7 +64,7 @@ export let ConditionBuilder = React.forwardRef(
initialState,
getConditionState,
getActionsState,
variant = 'sentence',
variant = NON_HIERARCHICAL_VARIANT,
actions,
translateWithId,
...rest
Expand Down Expand Up @@ -270,7 +273,7 @@ ConditionBuilder.propTypes = {
translateWithId: PropTypes.func,
/* TODO: add types and DocGen for all props. */
/**
* Provide the condition builder variant: sentence/ tree
* Provide the condition builder variant: Non-Hierarchical/ Hierarchical
*/
variant: PropTypes.oneOf(['tree', 'sentence']),
variant: PropTypes.oneOf(['Non-Hierarchical', 'Hierarchical']),
};
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export type ConditionBuilderProps = {
className?: string;
popOverSearchThreshold: number;
startConditionLabel: string;
variant?: 'sentence' | 'tree';
variant?: 'Non-Hierarchical' | 'Hierarchical';
translateWithId: (id: string) => string;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { ItemOption } from '../ConditionBuilderItem/ConditionBuilderItemOption/I
import {
blockClass,
connectorConfig,
HIERARCHICAL_VARIANT,
} from '../ConditionBuilderContext/DataConfigs';
import PropTypes from 'prop-types';
import { focusThisField } from '../utils/util';
Expand Down Expand Up @@ -52,7 +53,7 @@ const ConditionConnector = ({
onChange?.(op);
focusThisField(evt, conditionBuilderRef);
};
return variant == 'tree' ? (
return variant == HIERARCHICAL_VARIANT ? (
<span className={`${className} ${blockClass}__connector--disabled`}>
<ConditionBuilderButton label={operator} />
</span>
Expand Down
Loading

0 comments on commit 791e2b3

Please sign in to comment.