From d3f08bcd0b479a84e69fc29dfe03a931618168a7 Mon Sep 17 00:00:00 2001 From: Amal K Joy <153802538+amal-k-joy@users.noreply.github.com> Date: Mon, 21 Oct 2024 21:54:20 +0530 Subject: [PATCH] fix(conditionBuilder): release review changes1 (#6133) * fix(conditionBuilder): release review changes1 * fix: data correction for sample data --------- Co-authored-by: David Menendez Co-authored-by: Nandan Devadula <47176249+devadula-nandan@users.noreply.github.com> --- .../styles/_conditionBuilderItem.scss | 4 + .../ConditionBlock/ConditionBlock.tsx | 3 +- .../ConditionConnector.tsx | 4 - .../ConditionBuilderContent.tsx | 2 +- .../ConditionBuilderItemOption/ItemOption.tsx | 3 + .../ItemOptionForValueField.tsx | 4 + .../ConditionBuilder/assets/SampleData.js | 30 +-- .../ConditionBuilder/assets/sampleInput.js | 2 +- .../utils/handleKeyboardEvents.js | 212 +++++++++--------- .../components/ConditionBuilder/utils/util.js | 2 - 10 files changed, 139 insertions(+), 127 deletions(-) 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 5c93c2f71a..dd5a649325 100644 --- a/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss +++ b/packages/ibm-products-styles/src/components/ConditionBuilder/styles/_conditionBuilderItem.scss @@ -285,6 +285,10 @@ $colors: ( /* stylelint-disable-next-line carbon/motion-easing-use */ transition: transform motion(exit, productive) $duration-fast-02; } + .#{$block-class}__condition--interacting + .#{$block-class}__close-condition-wrapper { + z-index: 2; + } } .#{$block-class}__invalid-input { diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx index f736dce610..1e603b792b 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBlock/ConditionBlock.tsx @@ -254,7 +254,7 @@ const ConditionBlock = (props: ConditionBlockProps) => { onChange={(op) => onConnectorOperatorChange?.(op)} /> ) : ( -
+ '' )} {isStatement && ( @@ -344,6 +344,7 @@ const ConditionBlock = (props: ConditionBlockProps) => { renderIcon={Close} className={`${blockClass}__close-condition`} data-name="closeCondition" + wrapperClassName={`${blockClass}__close-condition-wrapper`} /> {/*
*/} diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx index 039d170664..b90118d725 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderConnector/ConditionConnector.tsx @@ -59,8 +59,6 @@ const ConditionConnector = ({ ) : ( - //
- - - //
); }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx index 47d5c14941..fd86105ec9 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderContent/ConditionBuilderContent.tsx @@ -150,7 +150,7 @@ const ConditionBuilderContent = ({ const addConditionGroupHandler = () => { const newGroup: ConditionGroup = { - statement: 'ifAll', // 'if|exclude if', + statement: 'ifAll', groupOperator: 'and', id: uuidv4(), conditions: [ diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOption.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOption.tsx index 83f421674c..b21bd01e0a 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOption.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOption.tsx @@ -85,6 +85,8 @@ export const ItemOption = ({ ); }; + const preventDefault = (evt) => evt.preventDefault(); + if (!allOptions) { return; } @@ -97,6 +99,7 @@ export const ItemOption = ({ labelText={clearSearchText} closeButtonLabelText={clearSearchText} onChange={onSearchChangeHandler} + onKeyDown={preventDefault} /> )} diff --git a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOptionForValueField.tsx b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOptionForValueField.tsx index 4dcb08f9b5..198b1696e9 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOptionForValueField.tsx +++ b/packages/ibm-products/src/components/ConditionBuilder/ConditionBuilderItem/ConditionBuilderItemOption/ItemOptionForValueField.tsx @@ -65,6 +65,7 @@ export const ItemOptionForValueField = ({ : []; useEffect(() => { + //this commented code is kept as intentional. Alternate approach to pass async options instead of getOptions callback. // if(rest['data-name'] == 'valueField'){ // const fetchData = async () => { // const response = await config.options(conditionState); @@ -153,6 +154,8 @@ export const ItemOptionForValueField = ({ ? conditionState.property : propertyText; }; + const preventDefault = (evt) => evt.preventDefault(); + if (!allOptions) { return ; } @@ -165,6 +168,7 @@ export const ItemOptionForValueField = ({ labelText={clearSearchText} closeButtonLabelText={clearSearchText} onChange={onSearchChangeHandler} + onKeyDown={preventDefault} /> )} diff --git a/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js b/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js index 718f608970..07814302f7 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js +++ b/packages/ibm-products/src/components/ConditionBuilder/assets/SampleData.js @@ -18,25 +18,25 @@ export const sampleDataStructure_Hierarchical = { { property: 'region', operator: 'is', - value: 'IL', + value: { id: 'India', label: 'India' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, //group object repeats @@ -48,25 +48,25 @@ export const sampleDataStructure_Hierarchical = { { property: 'region', operator: 'is', - value: 'IL', + value: { id: 'India', label: 'India' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, //group object repeats @@ -78,19 +78,19 @@ export const sampleDataStructure_Hierarchical = { { property: 'region', operator: 'is', - value: 'IL', + value: { id: 'India', label: 'India' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, //group object repeats @@ -108,13 +108,13 @@ export const sampleDataStructure_Hierarchical = { { property: 'continent', operator: 'is', - value: 'Asia', + value: { id: 'Asia', label: 'Asia' }, id: uuidv4(), }, { property: 'region', operator: 'is', - value: 'India', + value: { id: 'India', label: 'India' }, id: uuidv4(), }, { @@ -144,7 +144,7 @@ export const sampleDataStructure_nonHierarchical = { id: 'Africa', }, { - label: 'India', + label: { id: 'India', label: 'India' }, id: 'Ind', }, ], @@ -153,7 +153,7 @@ export const sampleDataStructure_nonHierarchical = { { property: 'delivery', operator: 'is', - value: 'processing', + value: { id: 'Processing', label: 'Processing' }, id: uuidv4(), }, { diff --git a/packages/ibm-products/src/components/ConditionBuilder/assets/sampleInput.js b/packages/ibm-products/src/components/ConditionBuilder/assets/sampleInput.js index 6dc5d0e819..6aeab8ae3d 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/assets/sampleInput.js +++ b/packages/ibm-products/src/components/ConditionBuilder/assets/sampleInput.js @@ -19,7 +19,7 @@ import { } from '@carbon/react/icons'; import CustomInput from './CustomInput'; -//keeping this , an alternative way to give support for dynamic options. +//keeping this commented code intentionally ,which is an alternative way to give support for dynamic options. //instead of supplying getOptions callback, we keep option property in inputConfig always as a async method instead to array as below. // export const inputDataForAsyncOptions = { // properties: [ diff --git a/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js b/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js index 897fa173b7..a87fc5ec94 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js +++ b/packages/ibm-products/src/components/ConditionBuilder/utils/handleKeyboardEvents.js @@ -57,120 +57,126 @@ const handleKeyPressForPopover = ( const isHoldingShiftKey = checkForHoldingKey(evt, 'shiftKey'); const isMultiSelect = parentContainer.querySelector('ul')?.dataset.multiSelect; + const isOptionInput = + parentContainer.querySelectorAll(`[role="option"]`)?.length; let allItems = []; - switch (key) { - case 'ArrowUp': - evt.preventDefault(); - //traverse through the popover options, search box, selectAll button - parentContainer - .querySelectorAll(`[role="option"]`) - .forEach((eachElem, index, allElements) => { - traverseReverse( - eachElem, - index, - allElements, - null, - null, - conditionBuilderRef - ); - }); - //scroll to top when we reach a the top of the list to make search box visible - if ( - Array.from(evt.target.closest('ul').querySelectorAll('li')).indexOf( - evt.target - ) === 1 - ) { - parentContainer.querySelector( - `.${blockClass}__popover-content-wrapper` - ).scrollTop = 0; - } - break; - case 'ArrowDown': - evt.preventDefault(); - //traverse through the popover options, search box, selectAll button - parentContainer - .querySelectorAll(`[role="option"]`) - .forEach((eachElem, index, allElements) => { - traverseClockVise( - eachElem, - index, - allElements, - null, - null, - conditionBuilderRef - ); - }); - - break; + if (key === 'Escape') { + //focus the corresponding field in which the popover is triggered from + focusThisField(evt, conditionBuilderRef); + evt.preventDefault(); + } - case 'Tab': - allItems = [ - ...Array.from( - parentContainer.querySelectorAll( - `.${blockClass}__selectAll-button,[role="searchbox"]` - ) - ), - parentContainer.querySelector(`[role="option"]`), - ]; - - allItems.forEach((eachElem, index, allElements) => { - if (isHoldingShiftKey) { - traverseReverse( - eachElem, - index, - allElements, - true, - true, - conditionBuilderRef - ); - } else { - traverseClockVise( - eachElem, - index, - allElements, - true, - true, - conditionBuilderRef - ); + if (isOptionInput) { + switch (key) { + case 'ArrowUp': + evt.preventDefault(); + //traverse through the popover options, search box, selectAll button + parentContainer + .querySelectorAll(`[role="option"]`) + .forEach((eachElem, index, allElements) => { + traverseReverse( + eachElem, + index, + allElements, + null, + null, + conditionBuilderRef + ); + }); + //scroll to top when we reach a the top of the list to make search box visible + if ( + Array.from( + evt.target.closest('ul')?.querySelectorAll('li') ?? [] + ).indexOf(evt.target) === 1 + ) { + parentContainer.querySelector( + `.${blockClass}__popover-content-wrapper` + ).scrollTop = 0; } - }); - evt.preventDefault(); - break; - case ' ': - if (isMultiSelect === 'true') { - if (document.activeElement.type !== 'button') { - //for button , enter key is click which already handled by framework, for other elements trigger click - document.activeElement?.click(); - } + break; + case 'ArrowDown': evt.preventDefault(); - } + //traverse through the popover options, search box, selectAll button + parentContainer + .querySelectorAll(`[role="option"]`) + .forEach((eachElem, index, allElements) => { + traverseClockVise( + eachElem, + index, + allElements, + null, + null, + conditionBuilderRef + ); + }); + + break; + + case 'Tab': + allItems = [ + ...Array.from( + parentContainer.querySelectorAll( + `.${blockClass}__selectAll-button,[role="searchbox"]` + ) + ), + parentContainer.querySelector(`[role="option"]`), + ]; + + allItems.forEach((eachElem, index, allElements) => { + if (isHoldingShiftKey) { + traverseReverse( + eachElem, + index, + allElements, + true, + true, + conditionBuilderRef + ); + } else { + traverseClockVise( + eachElem, + index, + allElements, + true, + true, + conditionBuilderRef + ); + } + }); + evt.preventDefault(); + break; - break; - case 'Enter': - if (isMultiSelect === 'true') { - if (document.activeElement.type !== 'button') { - //for button , enter key is click which already handled by framework, for other elements trigger click + case ' ': + if (isMultiSelect === 'true') { + if (document.activeElement.type !== 'button') { + //for button , enter key is click which already handled by framework, for other elements trigger click + document.activeElement?.click(); + } evt.preventDefault(); - document.activeElement?.click(); } - } else { - if (document.activeElement.type !== 'button') { - //for button , enter key is click which already handled by framework, else trigger click - focusThisField(evt, conditionBuilderRef); - document.activeElement?.click(); - } - } - break; - case 'Escape': - //focus the corresponding field in which the popover is triggered\ - focusThisField(evt, conditionBuilderRef); - break; + break; + case 'Enter': + if (isMultiSelect === 'true') { + if (document.activeElement.type !== 'button') { + //for button , enter key is click which already handled by framework, for other elements trigger click + evt.preventDefault(); + document.activeElement?.click(); + } + } else { + if (document.activeElement.type !== 'button') { + //for button , enter key is click which already handled by framework, else trigger click + focusThisField(evt, conditionBuilderRef); + document.activeElement?.click(); + } + } - default: - break; + break; + default: + break; + } } }; diff --git a/packages/ibm-products/src/components/ConditionBuilder/utils/util.js b/packages/ibm-products/src/components/ConditionBuilder/utils/util.js index b882f7aad1..08b6cae732 100644 --- a/packages/ibm-products/src/components/ConditionBuilder/utils/util.js +++ b/packages/ibm-products/src/components/ConditionBuilder/utils/util.js @@ -25,8 +25,6 @@ export const focusThisField = (evt, conditionBuilderRef) => { }; export const focusThisItem = (currentElement, conditionBuilderRef) => { setTimeout(() => { - //document.activeElement.setAttribute('tabindex', '-1'); - // currentElement.setAttribute('tabindex', '0'); manageTabIndexAndFocus(currentElement, conditionBuilderRef); }, 0); };