-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Moving inboxsearchcomposerv2 #51
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe recent updates encompass a broad enhancement of React components and utilities across the board, focusing on improved user interactions, data handling, and UI consistency. Significant changes include the introduction of new components, enhancements in form handling and validation, and refinements in UI rendering and state management. These changes aim to provide a more robust, customizable, and user-friendly experience in web applications. Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files ignored due to path filters (5)
Files selected for processing (35)
Files not reviewed due to errors (7)
Files skipped from review due to trivial changes (2)
Additional comments not posted (85)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
Out of diff range and nitpick comments (6)
react/ui-components/src/molecules/WorkflowStatusFilter.js (1)
45-45
: Consider adding more detailed comments or documentation regarding the future enhancement forformData
structure to help other developers understand the intended changes.react/ui-components/src/hoc/MobileView/MobileSearchComponent.js (1)
5-5
: Consider consolidating imports from the same directory to improve readability and reduce the number of import lines.react/ui-components/src/molecules/RenderFormFields.js (1)
17-17
: Consider using a more descriptive name for the componentApiCheckboxes
.The name
ApiCheckboxes
might be confusing as it suggests a specific relation to APIs, which may not be the case. Consider renaming it to something more generic likeCheckboxGroup
if it represents a group of checkboxes.react/ui-components/src/hoc/InboxSearchComposerV2.js (1)
46-46
: Clarify the purpose of theenable
state variable.The state variable
enable
is used in the component but it's not immediately clear what it controls. Consider renaming it to something more descriptive or adding a comment to clarify its purpose.react/example/.eslintcache (1)
1-1
: Ensure that the.eslintcache
file is not included in version control.Typically, cache files like
.eslintcache
should not be included in version control as they are generated dynamically and can vary between different environments. Consider adding this file to.gitignore
to prevent it from being tracked.react/ui-components/src/hoc/FormComposerV2.js (1)
36-38
: Consider removing commented-out code if it is no longer needed to keep the codebase clean.
return <Controller | ||
name={`${populators.name}`} | ||
control={control} | ||
defaultValue={formData?.[populators.name]} | ||
rules={{ required: populators?.isMandatory, ...populators.validation }} | ||
render={(props) => { | ||
return ( | ||
<div style={{ display: "grid", gridAutoFlow: "row" }}> | ||
array | ||
</div> | ||
); | ||
}} | ||
/> | ||
case "locationdropdown": | ||
return ( | ||
<Controller | ||
name={`${populators.name}`} | ||
control={control} | ||
defaultValue={formData?.[populators.name]} | ||
rules={{ required: populators?.isMandatory, ...populators.validation }} | ||
render={(props) => { | ||
return ( | ||
<div style={{ display: "grid", gridAutoFlow: "row" }}> | ||
<LocationDropdownWrapper | ||
props={props} | ||
populators={populators} | ||
formData={formData} | ||
inputRef={props.ref} | ||
errors={errors} | ||
setValue={setValue} | ||
/> | ||
</div> | ||
); | ||
}} | ||
/> | ||
); | ||
|
||
case "apidropdown": | ||
return ( | ||
<Controller | ||
name={`${populators.name}`} | ||
control={control} | ||
defaultValue={formData?.[populators.name]} | ||
rules={{ required: populators?.isMandatory, ...populators.validation }} | ||
render={(props) => { | ||
return ( | ||
<div style={{ display: "grid", gridAutoFlow: "row" }}> | ||
<ApiDropdown | ||
props={props} | ||
populators={populators} | ||
formData={formData} | ||
inputRef={props.ref} | ||
errors={errors} | ||
t={t} | ||
label={config?.label} | ||
type={type} | ||
onBlur={props.onBlur} | ||
value={props.value} | ||
onChange={props.onChange} | ||
config={populators} | ||
disable={config?.disable} | ||
errorStyle={errors?.[populators.name]} | ||
/> | ||
</div> | ||
); | ||
}} | ||
/> | ||
); | ||
case "multiselectdropdown": | ||
return ( | ||
<Controller | ||
name={`${populators.name}`} | ||
control={control} | ||
defaultValue={formData?.[populators.name]} | ||
rules={{ required: isMandatory }} | ||
render={(props) => { | ||
return ( | ||
<div style={{ display: "grid", gridAutoFlow: "row" }}> | ||
<MultiSelectDropdown | ||
options={populators?.options} | ||
optionsKey={populators?.optionsKey} | ||
props={props} | ||
isPropsNeeded={true} | ||
onSelect={(e) => { | ||
props.onChange(e?.map(row=>{return row?.[1] ? row[1] : null}).filter(e=>e)) | ||
}} | ||
selected={props?.value || []} | ||
defaultLabel={t(populators?.defaultText)} | ||
defaultUnit={t(populators?.selectedText)} | ||
config={populators} | ||
/> | ||
</div> | ||
); | ||
}} | ||
/> | ||
); | ||
default: | ||
return populators?.dependency !== false ? populators : null; | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fieldSelector
function is quite large and handles many different cases. Consider refactoring this into smaller, more manageable functions or components to improve readability and maintainability.
//remove Toast from 3s | ||
useEffect(()=>{ | ||
if(showErrorToast){ | ||
setTimeout(()=>{ | ||
closeToast(); | ||
},3000) | ||
} | ||
},[showErrorToast]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic to automatically close the toast after 3 seconds is a nice touch for user experience. However, consider making the timeout duration configurable through props to enhance flexibility.
const CheckBox = ({ onChange, label, value, disable, ref, checked, inputRef, pageType, style, index, isLabelFirst,customLabelMarkup, ...props }) => { | ||
const { t } = useTranslation() | ||
const userType = pageType || Digit.SessionStorage.get("userType"); | ||
let wrkflwStyle = props.styles; | ||
if (isLabelFirst) { | ||
return ( | ||
<div className="checkbox-wrap" style={wrkflwStyle ? wrkflwStyle : {}}> | ||
<p style={style ? style : null}> {index+1}.</p> | ||
<p className="label" style={{maxWidth: "80%", marginLeft: "10px"}}> | ||
{label} | ||
</p> | ||
<div> | ||
<input | ||
type="checkbox" | ||
className={userType === "employee" ? "input-emp" : ""} | ||
onChange={onChange} | ||
style={{ cursor: "pointer", left: "90%" }} | ||
value={value || label} | ||
{...props} | ||
ref={inputRef} | ||
disabled={disable} | ||
checked={checked} | ||
/> | ||
<p className={userType === "employee" ? "custom-checkbox-emp" : "custom-checkbox"} style={disable ? { opacity: 0.5 } : {left: "90%"}}> | ||
<CheckSvg /> | ||
</p> | ||
</div> | ||
</div> | ||
{!isLabelFirst ? ( | ||
<p className="label" style={{ maxWidth: "100%", width: "100%",marginRight:"0rem" }}> | ||
{customLabelMarkup ? ( | ||
<> | ||
<span>{t("COMMON_CERTIFY_ONE")}</span> | ||
<br /> | ||
<span> | ||
<b> {t("ES_COMMON_NOTE")}</b> | ||
{t("COMMON_CERTIFY_TWO")} | ||
</span> | ||
</> | ||
) : ( | ||
sentenceCaseLabel | ||
)} | ||
); | ||
} else { | ||
return ( | ||
<div className="checkbox-wrap" style={wrkflwStyle ? wrkflwStyle : {}}> | ||
<div> | ||
<input | ||
type="checkbox" | ||
className={userType === "employee" ? "input-emp" : ""} | ||
onChange={onChange} | ||
style={{ cursor: "pointer" }} | ||
value={value || label} | ||
{...props} | ||
ref={inputRef} | ||
disabled={disable} | ||
// {(checked ? (checked = { checked }) : null)} | ||
checked={checked} | ||
/> | ||
<p className={userType === "employee" ? "custom-checkbox-emp" : "custom-checkbox"} style={disable ? { opacity: 0.5 } : (props?.checkboxWidth ? {...props?.checkboxWidth} : null)}> | ||
{/* <img src={check} alt="" /> */} | ||
<CheckSvg /> | ||
</p> | ||
</div> | ||
<p className="label" style={style ? style : null}> | ||
{customLabelMarkup ? | ||
<> | ||
<p>{t("COMMON_CERTIFY_ONE")}</p> | ||
<br /> | ||
<p> | ||
<b> {t("ES_COMMON_NOTE")}</b>{t("COMMON_CERTIFY_TWO")} | ||
</p> | ||
</> : label} | ||
</p> | ||
) : null} | ||
</div> | ||
); | ||
</div> | ||
); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The modifications to support different label and checkbox placements based on the isLabelFirst
prop are well-implemented. Consider optimizing the prop handling to minimize unnecessary re-renders and improve performance.
const { t } = useTranslation(); | ||
const { fields, control, formData, errors, register, setValue, getValues, setError, clearErrors, apiDetails} = props | ||
|
||
const fieldSelector = (type, populators, isMandatory, disable = false, component, config) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Refactor fieldSelector
to improve readability and maintainability.
The fieldSelector
function is quite large and handles many different cases. Consider breaking it down into smaller, more manageable functions. Each case in the switch statement could potentially be a separate function. This would improve readability and maintainability.
//uiConfig.type === filter || sort | ||
//we need to sync browsersession and mobileSearchSession | ||
// const mobileSearchSession = Digit.Hooks.useSessionStorage(`MOBILE_SEARCH_MODAL_FORM_${uiConfig?.type}_${fullConfig?.label}`, | ||
// {...uiConfig?.defaultValues} | ||
// ); | ||
|
||
// const [sessionFormData, setSessionFormData, clearSessionFormData] = mobileSearchSession; | ||
const [session,setSession,clearSession] = browserSession || [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The commented-out session management code should either be removed or implemented if it's intended to be used. Leaving commented code can lead to confusion and clutter.
config.columns.map((column,idx) => { | ||
resultantObj[t(column.label)] = column.additionalCustomization ? Digit?.Customizations?.[apiDetails?.masterName]?.[apiDetails?.moduleName]?.additionalCustomizations(row,column?.label,column, _.get(row,column.jsonPath,""),t, searchResult) : String(_.get(row,column.jsonPath,"") ? column.translate? t(Digit.Utils.locale.getTransformedLocale(column.prefix?`${column.prefix}${_.get(row,column.jsonPath,"")}`:_.get(row,column.jsonPath,""))) : _.get(row,column.jsonPath,"") : t("ES_COMMON_NA")); | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider refactoring the nested ternary operations in convertRowToDetailCardData
to improve readability and maintainability.
}) | ||
.filter((e) => e) | ||
) | ||
: onChange( | ||
e | ||
?.map((row) => { | ||
return row?.[1] ? row[1] : null; | ||
}) | ||
.filter((e) => e) | ||
); | ||
}} | ||
disabled={disabled} | ||
selectedOption={value} | ||
defaultValue={value} | ||
t={t} | ||
errorStyle={errorStyle} | ||
additionalWrapperClass={additionalWrapperClass} | ||
innerStyles={config?.innerStyles} | ||
selected={props?.value || value} | ||
defaultLabel={t(config?.defaultText) } | ||
defaultUnit={t(config?.selectedText) || t("COMMON_SELECTED")} | ||
config={config} | ||
disable={disable} | ||
optionsDisable={config?.optionsDisable} | ||
/> | ||
); | ||
default: | ||
return null; | ||
} | ||
}; | ||
return <React.Fragment key={config.name}>{renderField()}</React.Fragment>; | ||
}; | ||
|
||
CustomDropdown.propTypes = { | ||
t: PropTypes.func.isRequired, | ||
config: PropTypes.shape({ | ||
mdmsConfig: PropTypes.shape({ | ||
masterName: PropTypes.string, | ||
moduleName: PropTypes.string, | ||
filter: PropTypes.object, | ||
select: PropTypes.string, | ||
localePrefix: PropTypes.string, | ||
}), | ||
name: PropTypes.string, | ||
optionsKey: PropTypes.string, | ||
styles: PropTypes.object, | ||
innerStyles: PropTypes.object, | ||
options: PropTypes.array, | ||
defaultValue: PropTypes.string, | ||
optionsCustomStyle: PropTypes.object, | ||
required: PropTypes.bool, | ||
}), | ||
inputRef: PropTypes.object, | ||
label: PropTypes.string, | ||
onChange: PropTypes.func, | ||
value: PropTypes.oneOfType([PropTypes.string, PropTypes.array]), | ||
errorStyle: PropTypes.object, | ||
disabled: PropTypes.bool, | ||
type: PropTypes.string, | ||
additionalWrapperClass: PropTypes.string, | ||
</div> : type === "radio" ? ( | ||
<RadioButtons | ||
inputRef={inputRef} | ||
style={{ display: "flex", justifyContent: "flex-start", gap: "3rem", ...config.styles }} | ||
options={data || config?.options || []} | ||
key={config.name} | ||
optionsKey={config?.optionsKey} | ||
value={value} | ||
onSelect={(e) => { | ||
onChange(e, config.name); | ||
}} | ||
disable={disable} | ||
selectedOption={value} | ||
defaultValue={value} | ||
t={t} | ||
errorStyle={errorStyle} | ||
additionalWrapperClass={additionalWrapperClass} | ||
innerStyles={config?.innerStyles} | ||
/> | ||
) : ( | ||
<Dropdown | ||
inputRef={inputRef} | ||
style={{ display: "flex", justifyContent: "space-between", ...config.styles }} | ||
option={data || config?.options || []} | ||
key={config.name} | ||
optionKey={config?.optionsKey} | ||
value={value} | ||
select={(e) => { | ||
onChange(e, config.name); | ||
}} | ||
disable={disable} | ||
selected={value || config.defaultValue} | ||
defaultValue={value || config.defaultValue} | ||
t={t} | ||
errorStyle={errorStyle} | ||
optionCardStyles={config?.optionsCustomStyle} | ||
/> | ||
) | ||
} | ||
</React.Fragment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider simplifying the event handling logic to make it more maintainable and less prone to errors, especially given the component's flexibility in rendering different types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of the new components were replaced with old can u check with i h
|
||
const getIconComponent = (iconName="")=>{ | ||
return require("@egovernments/digit-ui-svg-components")?.[iconName]; | ||
return require("@egovernments/digit-ui-react-components")?.[iconName]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Keep as svg
disabled, | ||
variant, | ||
}) => { | ||
const MultiSelectDropdown = ({ options, optionsKey, selected = [], onSelect, defaultLabel = "", defaultUnit = "",BlockNumber=1,isOBPSMultiple=false,props={},isPropsNeeded=false,ServerStyle={}, isSurvey=false,placeholder, disable=false,config}) => { | ||
const [active, setActive] = useState(false); | ||
const [searchQuery, setSearchQuery] = useState(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert to old component
<div className="digit-tag-container"> | ||
{alreadyQueuedSelectedState.length > 0 && | ||
alreadyQueuedSelectedState.map((value, index) => { | ||
if (!value.propsData[1]?.options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There were too many changes
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Refactor