Enhancements of Components #52
Conversation
|
Important Auto Review SkippedReview was skipped due to path filters Files ignored due to path filters (1)
You can disable this status message by setting the WalkthroughThe recent updates across various CSS components in the Changes
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Out of diff range and nitpick comments (5)
react/ui-components/README.md (1)
61-61: The changelog entry0.0.1-beta.12 enhancements of componentsis vague. Consider providing more detailed descriptions of the enhancements for better clarity.react/css/README.md (1)
Line range hint
1-84: Update the README file for clarity and completeness.Consider updating the TODO comment at the beginning of the file to provide more specific information about what needs to be updated. Additionally, ensure that the installation and usage instructions are clear and accurate.
react/ui-components/src/hoc/FormComposerV2.js (1)
268-268: Consider using a more descriptive placeholder transformation key.The placeholder transformation key "TOSENTENCECASE" might not be clear to other developers or future maintainers. Consider using a more descriptive key or adding a comment explaining its purpose.
react/ui-components/src/atoms/TextInput.js (1)
268-268: Ensure consistent placeholder transformation.The placeholder is transformed to sentence case using the
StringManipulatorfunction. Ensure that this transformation is consistent across all instances where placeholders are used in the application to maintain a uniform user experience.react/modules/sample/src/pages/employee/Sample.js (1)
Line range hint
217-900: Consistency in button labels with excessively long character counts.- "PrimaryWithmorethansixtyfourcharactersPrimaryWithsixtyfourcharacters" + "PrimaryWithMoreThanSixtyFourCharacters"Consider reducing the character count in button labels to enhance readability and maintain UI consistency. Adjust the labels to be more concise and clear.
| function DocPreview({ file, ...props }) { | ||
| const { t } = useTranslation(); | ||
|
|
||
| const documents = file | ||
| ? [ | ||
| { | ||
| fileName: file?.name, | ||
| }, | ||
| ] | ||
| : null; | ||
|
|
||
| return ( | ||
| <PopUp className="campaign-data-preview" style={{ flexDirection: "column" }}> | ||
| {/* <div style={{ display: "flex", justifyContent: "space-between", marginLeft: "2.5rem", marginRight: "2.5rem", marginTop: "2.5rem" }}> | ||
| <Button | ||
| label={t("BACK")} | ||
| variation="secondary" | ||
| icon={<SVG.ArrowBackIos styles={{ height: "1.25rem", width: "1.25rem" }} fill="#F47738" />} | ||
| type="button" | ||
| className="workbench-download-template-btn" | ||
| onButtonClick={() => props?.onBack()} | ||
| /> | ||
| <Button | ||
| label={t("WBH_DOWNLOAD")} | ||
| variation="secondary" | ||
| icon={<DownloadIcon styles={{ height: "1.25rem", width: "1.25rem" }} fill="#F47738" />} | ||
| type="button" | ||
| className="workbench-download-template-btn" | ||
| onButtonClick={() => props?.onDownload()} | ||
| /> | ||
| </div> */} | ||
| <div className="campaign-popup-module" style={{ marginTop: "1.5rem" }}> | ||
| <DocViewer | ||
| style={{ height: "80vh", overflowY: "hidden" }} | ||
| theme={{ | ||
| primary: "#F47738", | ||
| secondary: "#feefe7", | ||
| tertiary: "#feefe7", | ||
| textPrimary: "#0B0C0C", | ||
| textSecondary: "#505A5F", | ||
| textTertiary: "#00000099", | ||
| disableThemeScrollbar: true, | ||
| }} | ||
| documents={documents} | ||
| pluginRenderers={DocViewerRenderers} | ||
| /> | ||
| </div> | ||
| </PopUp> | ||
| ); | ||
| } | ||
|
|
||
| export default DocPreview; |
There was a problem hiding this comment.
Consider removing or revising commented-out code.
If the commented-out code (lines 22-39) is no longer needed, consider removing it to clean up the file. If it might be used in the future, consider adding a TODO comment with more context.
| @@ -265,6 +265,7 @@ export const FormComposer = (props) => { | |||
| ? { ...wrapperStyles, ...field?.populators?.customStyle } | |||
| : { border: "none", background: "white", ...field?.populators?.customStyle } | |||
| } | |||
| className={props?.labelfielddirectionvertical ? "vertical" : ""} | |||
There was a problem hiding this comment.
Optimize useCallback dependencies.
The formFields useCallback hook does not include all its dependencies. Consider adding all external variables and functions used inside the callback to its dependency array to avoid stale closures and bugs related to outdated captures.
Simplify the form rendering logic.
The main return block of the FormComposer component is quite complex and hard to read due to nested conditional rendering and a large number of JSX elements. Consider breaking down this component into smaller subcomponents or using a more declarative approach to improve readability and maintainability.
| @@ -371,7 +371,7 @@ const Dropdown = (props) => { | |||
| )} | |||
| {(props.variant === "nesteddropdown" ? flattenedOptions : filteredOption) && | |||
| (props.variant === "nesteddropdown" ? flattenedOptions : filteredOption).length === 0 && ( | |||
| <div className={`cp digit-dropdown-item ${props.variant ? props?.variant : ""}`} key={"-1"} onClick={() => {}}> | |||
| <div className={`cp digit-dropdown-item unsuccessfulresults ${props.variant ? props?.variant : ""}`} key={"-1"} onClick={() => {}}> | |||
There was a problem hiding this comment.
Handle empty results more gracefully.
The handling of "NO RESULTS FOUND" could be improved by providing a more user-friendly message or additional guidance when no options match the filter criteria. Consider adding suggestions or a clearer call to action.
| (window.innerWidth / window.innerHeight <= 9/16) | ||
| ); | ||
| const onResize = () => { | ||
| if (window.innerWidth <= 480) { | ||
| if (window.innerWidth / window.innerHeight <= 9/16) { |
There was a problem hiding this comment.
Consider using a more robust method for determining if the device is in a mobile view, such as CSS media queries or a dedicated library, to improve accuracy and maintainability.
| try { | ||
| const components = require("@egovernments/digit-ui-svg-components"); | ||
| const DynamicIcon = components?.[iconReq]; | ||
| if (DynamicIcon) { | ||
| const svgElement = DynamicIcon({ | ||
| width: width, | ||
| height: height, | ||
| fill: iconFill, | ||
| className: `digit-button-customIcon ${props?.size ? props?.size : ""} ${props?.variation ? props?.variation : ""} ` | ||
| }); | ||
| return svgElement; | ||
| } else { | ||
| console.log("Icon not found"); | ||
| return null; | ||
| } | ||
| } catch (error) { | ||
| console.error("Icon not found"); | ||
| return null; | ||
| } |
There was a problem hiding this comment.
Consider handling the dynamic import of SVG components more efficiently, possibly by using a context or provider pattern to avoid repeated imports and error handling in each render cycle.
There was a problem hiding this comment.
Actionable comments posted: 0
Out of diff range and nitpick comments (4)
react/css/README.md (4)
Line range hint
1-1: Clarify the TODO comment for better actionability.Consider specifying what exactly needs to be updated to help other developers understand the required actions more clearly.
Line range hint
13-15: Improve the grammatical structure of the Limitation section.- This Package is more specific to DIGIT-UI's can be used across mission's + This package is specifically designed for DIGIT-UI and can be used across various missions.
Line range hint
19-25: Clarify and correct grammar in the Usage section.- After adding the dependency make sure you have this dependency in + After adding the dependency, ensure it is listed in
45-45: Format the Changelog entries for better readability.Consider using a bulleted list or ensuring each entry is on a new line to enhance clarity and readability.
| @@ -27,7 +27,8 @@ | |||
| } | |||
|
|
|||
| &:hover { | |||
| @apply border-2 border-primary-main; | |||
| @apply border-2 ; | |||
| border-color: theme(digitv2.lightTheme.primary-1); | |||
There was a problem hiding this comment.
what is primary-1 color ?
There was a problem hiding this comment.
Updated the color typography : the primary-orange is renamed to primary-1 and the color code have been changed.
| "primary-bluegray": "#0B4B66", | ||
| "primary-orangebg": "#FEEFE7", | ||
|
|
||
| "primary-1": "#C84C0E", |
There was a problem hiding this comment.
why primary 1 2 ?
react/ui-components/package.json
Outdated
| @@ -60,7 +60,9 @@ | |||
| "react-date-range": "1.3.0", | |||
| "react-hook-form": "6.15.8", | |||
| "react-i18next": "11.16.2", | |||
| "react-table": "7.7.0" | |||
| "react-table": "7.7.0", | |||
| "react-drag-drop-files": "^2.3.10", | |||
There was a problem hiding this comment.
dont use ^ keep fixed version
| @@ -229,7 +229,7 @@ const Dropdown = (props) => { | |||
| return ( | |||
| <div | |||
| className={`cp digit-dropdown-item ${props.variant ? props?.variant : ""}`} | |||
| style={index === optionIndex ? { opacity: 1, backgroundColor: "#FFFAF7", border: "0.5px solid #F47738" } : {}} | |||
| style={index === optionIndex ? { opacity: 1, backgroundColor: "#FFFAF7", border: "0.5px solid #C84C0E" } : {}} | |||
There was a problem hiding this comment.
why inline style , apply through class
| @@ -250,7 +250,7 @@ const Dropdown = (props) => { | |||
| /> | |||
| ) : null} | |||
| <div className="option-des-container"> | |||
| <div style={{ display: "flex", gap: "0.25rem", alignItems: "center", width: "100%", minHeight: "18px" }}> | |||
| <div style={{ display: "flex", gap: "0.25rem", alignItems: "center", width: "100%" }}> | |||
There was a problem hiding this comment.
why inline style , apply through class
| {props?.showAsTags && ( | ||
| <div className="digit-tag-container" style={{ marginTop: "0px" }}> | ||
| {uploadedFiles?.map((file, index) => { | ||
| console.log(file, "file"); |
There was a problem hiding this comment.
remove console logs
| addSelectAllCheck = false, | ||
| addCategorySelectAllCheck = false, | ||
| selectAllLabel = "", | ||
| categorySelectAllLabel = "" |
There was a problem hiding this comment.
Add PropTypes validation for new props to ensure type safety.
| useEffect(() => { | ||
| const initialCategorySelectedState = options.reduce((acc, category) => { | ||
| if (category.options) { | ||
| acc[category.code] = category.options.every((option) => | ||
| alreadyQueuedSelectedState.some( | ||
| (selectedOption) => selectedOption.code === option.code | ||
| ) | ||
| ); | ||
| } | ||
| return acc; | ||
| }, {}); | ||
| setCategorySelected(initialCategorySelectedState); | ||
| }, [options, alreadyQueuedSelectedState]); |
There was a problem hiding this comment.
Optimize the calculation of initialCategorySelectedState by memoizing the result to avoid unnecessary recalculations on each render.
| const isAlreadySelected = alreadyQueuedSelectedState.some( | ||
| (selectedOption) => selectedOption.code === option.code | ||
| ); | ||
| if (!isAlreadySelected) { | ||
| dispatch({ type: "ADD_TO_SELECTED_EVENT_QUEUE", payload: [null, option] }); | ||
| dispatch({ | ||
| type: "ADD_TO_SELECTED_EVENT_QUEUE", | ||
| payload: [null, option], | ||
| }); | ||
| } else { | ||
| dispatch({ type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: [null, option] }); | ||
| dispatch({ | ||
| type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", | ||
| payload: [null, option], | ||
| }); | ||
| const parentOption = findParentOption(option, options); | ||
| if (parentOption) { | ||
| dispatch({ type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", payload: [null, parentOption] }); | ||
| dispatch({ | ||
| type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", | ||
| payload: [null, parentOption], | ||
| }); |
There was a problem hiding this comment.
Refactor onSelectToAddToQueue to simplify the nested dispatch calls and improve readability.
| const handleCategorySelection = (parentOption) => { | ||
| const childoptions = parentOption.options; | ||
| if (!categorySelected[parentOption.code]) { | ||
| childoptions?.forEach((option) => { | ||
| dispatch({ | ||
| type: "ADD_TO_SELECTED_EVENT_QUEUE", | ||
| payload: [null, option], | ||
| }); | ||
| }); | ||
| } else { | ||
| childoptions?.forEach((option) => { | ||
| dispatch({ | ||
| type: "REMOVE_FROM_SELECTED_EVENT_QUEUE", | ||
| payload: [null, option], | ||
| }); | ||
| }); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Optimize handleCategorySelection by reducing the number of dispatch calls when toggling the selection state.
Summary by CodeRabbit
New Features
Style Updates
Documentation
index.htmlfor consistency.