From 791e2b31549f3f4480cac2fc142e550b5e12ea31 Mon Sep 17 00:00:00 2001 From: Amal K Joy <153802538+amal-k-joy@users.noreply.github.com> Date: Thu, 22 Aug 2024 10:34:25 +0530 Subject: [PATCH] feat(Conditionbuilder): renaming both variants to Hierarchical and Non-Hierarchical (#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 --- .../ConditionBuilder/_condition-builder.scss | 2 +- .../styles/_conditionBuilderItem.scss | 2 +- .../ConditionBlock/ConditionBlock.tsx | 12 +++-- .../ConditionBuilder.stories.jsx | 49 ++++++++++--------- .../ConditionBuilder/ConditionBuilder.test.js | 44 +++++++++-------- .../ConditionBuilder/ConditionBuilder.tsx | 11 +++-- .../ConditionBuilder.types.ts | 2 +- .../ConditionConnector.tsx | 3 +- .../ConditionBuilderContent.tsx | 16 ++++-- .../ConditionBuilderProvider.tsx | 2 +- .../ConditionBuilderContext/DataConfigs.js | 2 + .../translationObject.js | 1 + .../ConditionGroupBuilder.tsx | 16 +++--- .../ConditionBuilder/assets/SampleData.js | 4 +- .../utils/handleKeyboardEvents.js | 19 ++++--- 15 files changed, 108 insertions(+), 77 deletions(-) diff --git a/packages/ibm-products-styles/src/components/ConditionBuilder/_condition-builder.scss b/packages/ibm-products-styles/src/components/ConditionBuilder/_condition-builder.scss index 630d841a58..ef02c1ffc6 100644 --- a/packages/ibm-products-styles/src/components/ConditionBuilder/_condition-builder.scss +++ b/packages/ibm-products-styles/src/components/ConditionBuilder/_condition-builder.scss @@ -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; diff --git a/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss b/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss index 9eacd051f1..82bb042482 100644 --- a/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss +++ b/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss @@ -231,7 +231,7 @@ $colors: ( } } -.#{$block-class}__tree { +.#{$block-class}__Hierarchical { .#{$block-class}__condition-wrapper > :nth-child(n + 3) { flex-basis: 100%; } diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx index e8f7ef34bf..f7940492ea 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx @@ -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'; @@ -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); @@ -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, @@ -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, @@ -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`} diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.stories.jsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.stories.jsx index 70747d7f41..baa53283dc 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.stories.jsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.stories.jsx @@ -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, @@ -223,7 +227,7 @@ export const conditionBuilder = ConditionBuilderTemplate.bind({}); conditionBuilder.storyName = 'Condition Builder'; conditionBuilder.args = { inputConfig: inputData, - variant: 'sentence', + variant: NON_HIERARCHICAL_VARIANT, }; export const conditionBuilderDynamicOptions = ConditionBuilderTemplate.bind({}); @@ -231,7 +235,7 @@ conditionBuilderDynamicOptions.storyName = 'With dynamic options'; conditionBuilderDynamicOptions.args = { inputConfig: inputDataDynamicOptions, getOptions: getOptions, - variant: 'sentence', + variant: NON_HIERARCHICAL_VARIANT, }; export const conditionBuilderWithInitialState = ConditionBuilderTemplate.bind( @@ -239,9 +243,9 @@ 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, }; @@ -249,35 +253,36 @@ 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) => {}, }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.test.js b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.test.js index 7d4c0804bd..7d92052f56 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.test.js +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.test.js @@ -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; @@ -33,7 +37,7 @@ const defaultProps = { startConditionLabel: 'Add condition', popOverSearchThreshold: 4, getConditionState: () => {}, - variant: 'sentence', + variant: NON_HIERARCHICAL_VARIANT, }; const inputConfigOptionType = { @@ -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( @@ -373,7 +377,7 @@ describe(componentName, () => { ); //start builder @@ -603,7 +607,7 @@ describe(componentName, () => { ); @@ -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( ); @@ -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( ); @@ -842,7 +846,7 @@ describe(componentName, () => { ); @@ -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: [ @@ -935,7 +939,7 @@ describe(componentName, () => { ); @@ -1204,7 +1208,7 @@ describe(componentName, () => { render( @@ -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( { expect(screen.getByText('Add condition')).toHaveFocus(); }); - //for tree variant + //for Hierarchical variant it('add and remove conditions using keyboard', async () => { render( ); @@ -1515,9 +1519,9 @@ describe(componentName, () => { render( ); diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.tsx index 3bdffac77f..cc0ee87b1f 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.tsx @@ -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'; @@ -61,7 +64,7 @@ export let ConditionBuilder = React.forwardRef( initialState, getConditionState, getActionsState, - variant = 'sentence', + variant = NON_HIERARCHICAL_VARIANT, actions, translateWithId, ...rest @@ -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']), }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.types.ts b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.types.ts index 288933980f..fc321c64c5 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.types.ts +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilder.types.ts @@ -181,7 +181,7 @@ export type ConditionBuilderProps = { className?: string; popOverSearchThreshold: number; startConditionLabel: string; - variant?: 'sentence' | 'tree'; + variant?: 'Non-Hierarchical' | 'Hierarchical'; translateWithId: (id: string) => string; }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx index 204d008935..2d56ce1879 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx @@ -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'; @@ -52,7 +53,7 @@ const ConditionConnector = ({ onChange?.(op); focusThisField(evt, conditionBuilderRef); }; - return variant == 'tree' ? ( + return variant == HIERARCHICAL_VARIANT ? ( diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx index 101886070b..65c7df2abb 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx @@ -14,7 +14,10 @@ import { ConditionBuilderContext, emptyState, } from '../ConditionBuilderContext/ConditionBuilderProvider'; -import { blockClass } from '../ConditionBuilderContext/DataConfigs'; +import { + blockClass, + HIERARCHICAL_VARIANT, +} from '../ConditionBuilderContext/DataConfigs'; import { ConditionBuilderButton } from '../ConditionBuilderButton/ConditionBuilderButton'; import uuidv4 from '../../../global/js/utils/uuidv4'; import ConditionPreview from '../ConditionPreview/ConditionPreview'; @@ -52,9 +55,14 @@ const ConditionBuilderContent = ({ const [showConditionGroupPreview, setShowConditionGroupPreview] = useState(false); - const [addConditionGroupText, conditionHeadingText] = useTranslations([ + const [ + addConditionGroupText, + conditionHeadingText, + conditionBuilderHierarchicalText, + ] = useTranslations([ 'addConditionGroupText', 'conditionHeadingText', + 'conditionBuilderHierarchicalText', ]); const showConditionGroupPreviewHandler = () => { setShowConditionGroupPreview(true); @@ -172,7 +180,7 @@ const ConditionBuilderContent = ({
{rootState && rootState?.groups?.map((eachGroup, groupIndex) => ( @@ -202,7 +210,7 @@ const ConditionBuilderContent = ({ ))} {/* button to add a new group */} - {variant == 'tree' && ( + {variant == HIERARCHICAL_VARIANT && (
(null); const onRemoveHandler = (conditionId, evt, conditionIndex) => { if (group && group.conditions && group.conditions.length > 1) { - variant == 'tree' - ? handleFocusOnCloseTree(evt) + variant == HIERARCHICAL_VARIANT + ? handleFocusOnCloseHierarchical(evt) : handleFocusOnClose(evt, conditionIndex); if (!checkGroupHaveCondition(group.conditions, conditionId)) { @@ -178,7 +180,7 @@ const ConditionGroupBuilder = ({ ); } }; - const handleFocusOnCloseTree = (evt) => { + const handleFocusOnCloseHierarchical = (evt) => { //getting the current aria-level and aria-posinset. const currentLevel = evt.currentTarget ?.closest('[role="row"]') @@ -314,7 +316,7 @@ const ConditionGroupBuilder = ({ }); }; - const getSentenceVariant = () => { + const getNonHierarchicalVariant = () => { return (
{ + const getHierarchicalVariant = () => { return (
- {variant == 'tree' && getTreeVariant()} - {variant == 'sentence' && getSentenceVariant()} + {variant == HIERARCHICAL_VARIANT && getHierarchicalVariant()} + {variant == NON_HIERARCHICAL_VARIANT && getNonHierarchicalVariant()} ); }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js b/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js index a6a8db7a01..b4971c0c22 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js +++ b/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js @@ -7,7 +7,7 @@ import uuidv4 from '../../../global/js/utils/uuidv4'; -export const sampleDataStructure_tree = { +export const sampleDataStructure_Hierarchical = { operator: 'or', groups: [ { @@ -128,7 +128,7 @@ export const sampleDataStructure_tree = { ], }; -export const sampleDataStructure_sentence = { +export const sampleDataStructure_nonHierarchical = { groups: [ { groupOperator: 'and', //'and|or', diff --git a/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js b/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js index b65522584d..b5a94f6535 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js +++ b/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js @@ -5,7 +5,10 @@ * LICENSE file in the root directory of this source tree. */ -import { blockClass } from '../ConditionBuilderContext/DataConfigs'; +import { + blockClass, + HIERARCHICAL_VARIANT, +} from '../ConditionBuilderContext/DataConfigs'; import { checkForHoldingKey, focusThisField, @@ -177,7 +180,7 @@ const handleKeyPressForMainContent = (evt, conditionBuilderRef, variant) => { switch (evt.key) { case 'ArrowRight': evt.preventDefault(); - if (variant == 'tree') { + if (variant == HIERARCHICAL_VARIANT) { let allCellsInRow = Array.from( evt.target .closest('[role="row"]') @@ -185,7 +188,7 @@ const handleKeyPressForMainContent = (evt, conditionBuilderRef, variant) => { ); if (allCellsInRow.length === 1) { evt.target = evt.target.closest('[role="row"]'); - handleRowNavigationTree(evt, conditionBuilderRef, variant); + handleRowNavigationHierarchical(evt, conditionBuilderRef, variant); //focus next row } else if (evt.target.getAttribute('role') == 'row') { //when current focus is on a row, then we need to enter inside and focus the first cell of that row @@ -210,7 +213,7 @@ const handleKeyPressForMainContent = (evt, conditionBuilderRef, variant) => { break; case 'ArrowLeft': evt.preventDefault(); - if (variant == 'tree') { + if (variant == HIERARCHICAL_VARIANT) { if (evt.target.getAttribute('role') !== 'row') { //when any cell is focussed, arrow left will select the previous cell or current row @@ -241,8 +244,8 @@ const handleKeyPressForMainContent = (evt, conditionBuilderRef, variant) => { case 'ArrowUp': case 'ArrowDown': evt.preventDefault(); - if (variant == 'tree') { - handleRowNavigationTree(evt, conditionBuilderRef, variant); + if (variant == HIERARCHICAL_VARIANT) { + handleRowNavigationHierarchical(evt, conditionBuilderRef, variant); } else { handleRowNavigation(evt, conditionBuilderRef, variant); } @@ -282,7 +285,7 @@ const handleRowNavigation = (evt, conditionBuilderRef, variant) => { conditionBuilderRef ); }; -const handleRowNavigationTree = (evt, conditionBuilderRef, variant) => { +const handleRowNavigationHierarchical = (evt, conditionBuilderRef, variant) => { const rows = getRows(conditionBuilderRef); const currentRowIndex = getRowIndex(evt.target, conditionBuilderRef); let nextRowIndex = currentRowIndex; @@ -341,7 +344,7 @@ const navigateToNextRowCell = ( nextRow?.querySelector(`[data-name="${itemName}"]`), conditionBuilderRef ); - } else if (variant === 'tree') { + } else if (variant === HIERARCHICAL_VARIANT) { //when the next row is a if statement , then that row is focused. From any cell of last row of an group , arrow down select the next row (if) manageTabIndexAndFocus(nextRow, conditionBuilderRef); }