[OSDEV-1132] implement thanks for submitting screen FE

[VadimKovalenkoSNF]

Implement OSDEV-1132
Created FE for the "thanks for submitting screen" when submit new production location.

[barecheck]

Dedupe Hub App | Unittest test suite - Code coverage report

Total: 56.14%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Countries App | Unittest test suite - Code coverage report

Total: 100%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Contricleaner App | Unittest test suite - Code coverage report

Total: 98.91%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[barecheck]

Django App | Unittest test suite - Code coverage report

Total: 79.95%

Your code coverage diff: 0.00% ▴

✅ All code changes are covered

[roman-stolar]

LGTM

[mazursasha1990]

Left several questions.

[coderabbitai]

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (4)

src/react/src/components/DialogTooltip.jsx (2)

7-42: Consider memoizing the component for better performance.

The component is well-structured, but since it's likely to be reused in multiple places (e.g., in ProductionLocationDialog), consider wrapping it with React.memo to prevent unnecessary re-renders.

-const DialogTooltip = ({ text, childComponent, classes }) => {
+const DialogTooltip = React.memo(({ text, childComponent, classes }) => {
     const [arrowRef, setArrowRef] = useState(null);
     return (
         // ... component implementation
     );
-};
+});

---

44-56: Enhance PropTypes validation for classes prop.

The classes prop uses generic shape({}) for all style properties. Consider defining specific PropTypes for each style property to better document the expected types and catch potential styling issues.

     classes: shape({
-        arrow: shape({}).isRequired,
-        tooltipStyles: shape({}).isRequired,
-        popperStyles: shape({}).isRequired,
-        placementLeft: shape({}).isRequired,
-        placementRight: shape({}).isRequired,
-        placementTop: shape({}).isRequired,
-        placementBottom: shape({}).isRequired,
+        arrow: shape({
+            position: string,
+            fontSize: oneOfType([string, number]),
+            width: string,
+            height: string
+        }).isRequired,
+        tooltipStyles: shape({
+            fontSize: string,
+            backgroundColor: string
+        }).isRequired,
+        popperStyles: object.isRequired,
+        placementLeft: shape({
+            margin: string
+        }).isRequired,
+        placementRight: shape({
+            margin: string
+        }).isRequired,
+        placementTop: shape({
+            margin: string
+        }).isRequired,
+        placementBottom: shape({
+            margin: string
+        }).isRequired,
     }).isRequired,

src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)

17-31: Extract and optimize helper components.

The helper components should be:

1. Moved to separate files for better maintainability
2. Memoized to prevent unnecessary re-renders
3. Have their own PropTypes validation

Create new files for these components:

// src/react/src/components/Contribute/InfoIcon.jsx
import React from 'react';
import PropTypes from 'prop-types';
import InfoOutlinedIcon from '@material-ui/icons/InfoOutlined';

const InfoIcon = React.memo(({ className }) => (
    <InfoOutlinedIcon className={className} />
));

InfoIcon.propTypes = {
    className: PropTypes.string.isRequired
};

export default InfoIcon;

// src/react/src/components/Contribute/ClaimButton.jsx
import React from 'react';
import PropTypes from 'prop-types';
import Button from '@material-ui/core/Button';

const ClaimButton = React.memo(({ classes }) => (
    <span className={`${classes.claimTooltipWrapper}`}>
        <Button
            variant="contained"
            disabled
            className={`${classes.button} ${classes.claimButton}`}
        >
            Continue to Claim
        </Button>
    </span>
));

ClaimButton.propTypes = {
    classes: PropTypes.shape({
        claimTooltipWrapper: PropTypes.string.isRequired,
        button: PropTypes.string.isRequired,
        claimButton: PropTypes.string.isRequired
    }).isRequired
};

export default ClaimButton;

src/react/src/util/styles.js (1)

477-519: Add JSDoc documentation for better type safety.

The function would benefit from proper documentation and type hints.

+/**
+ * Generates styles for tooltip arrows
+ * @param {string} color - CSS color value for the arrow
+ * @param {string} selector - CSS selector for the arrow element
+ * @returns {Object} Object containing styles for different arrow placements
+ */
 const arrowGenerator = (color, selector) => ({

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 25308d7 and 1967a45.

📒 Files selected for processing (3)

• src/react/src/components/Contribute/ProductionLocationDialog.jsx (1 hunks)
• src/react/src/components/DialogTooltip.jsx (1 hunks)
• src/react/src/util/styles.js (2 hunks)

🧰 Additional context used

📓 Learnings (1)

src/react/src/components/DialogTooltip.jsx (1)

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/DialogTooltip.jsx:0-0
Timestamp: 2024-12-03T10:29:02.409Z
Learning: In the `DialogTooltip` component (`src/react/src/components/DialogTooltip.jsx`), there are existing tooltip tests that cover common cases, and further detailed tests are not necessary.

🔇 Additional comments (6)

src/react/src/components/DialogTooltip.jsx (2)

1-5: LGTM! Well-organized imports.

The imports are properly structured using named imports, which is good for tree-shaking.

---

58-58: LGTM! Proper usage of Material-UI's HOC pattern.

The export statement correctly uses the withStyles HOC to inject styles into the component.

src/react/src/components/Contribute/ProductionLocationDialog.jsx (2)

183-185: LGTM! Proper usage of Material-UI's HOC pattern.

The export statement correctly uses the withStyles HOC to inject styles into the component.

---

164-166: ⚠️ Potential issue

Replace console.log with proper handler.

Remove the console.log and implement proper error handling or event handling.

-onClick={() => {
-    console.log('submit another location');
-}}
+onClick={onSubmitAnother}

⛔ Skipped due to learnings

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.

src/react/src/util/styles.js (2)

522-557: LGTM! Well-structured tooltip styles.

The styles are properly organized and follow Material-UI's styling patterns.

---

Line range hint 648-683: LGTM! Consistent use of color constants and theme patterns.

The styles maintain consistency with the codebase's color scheme and Material-UI theme patterns.

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)

src/react/src/util/styles.js (1)

559-645: Consider grouping related styles and extracting magic numbers.

While the styles are well-implemented, consider these improvements:

1. Group related styles (e.g., button styles, typography styles) into separate objects for better organization
2. Extract magic numbers into named constants (e.g., PADDING_LARGE: '60px')

Example grouping:

 export const makeProductionLocationDialogStyles = theme => {
+    const typography = Object.freeze({
+        label: Object.freeze({
+            fontSize: '14px',
+            textTransform: 'uppercase',
+            fontWeight: theme.typography.fontWeightExtraBold,
+        }),
+        titleContentStyle: Object.freeze({
+            fontSize: '32px',
+            textAlign: 'center',
+            fontWeight: theme.typography.fontWeightBold,
+            lineHeight: 1,
+        }),
+        // ... other typography styles
+    });
+
+    const buttons = Object.freeze({
+        button: Object.freeze({
+            fontWeight: 'bold',
+            textTransform: 'none',
+            paddingLeft: '30px',
+            paddingRight: '30px',
+            boxShadow: 'none',
+            [theme.breakpoints.down('md')]: {
+                width: '100%',
+                marginBottom: '16px',
+            },
+        }),
+        claimButton: Object.freeze({
+            backgroundColor: COLOURS.NAVIGATION,
+        }),
+    });
+
     return Object.freeze({
+        ...typography,
+        ...buttons,
         // ... other styles
     });
 };

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 1967a45 and 48e8a72.

📒 Files selected for processing (1)

• src/react/src/util/styles.js (2 hunks)

🔇 Additional comments (4)

src/react/src/util/styles.js (4)

477-519: LGTM! Well-structured arrow generator function.

The refactored arrowGenerator function with the new selector parameter improves reusability while maintaining immutability through Object.freeze.

---

522-557: LGTM! Well-organized tooltip styles.

The new makeDialogTooltipStyles function maintains consistency with the codebase's styling patterns and properly handles different tooltip placements.

---

647-659: LGTM! Clean and semantic button styles.

The updated back button styles are well-organized and make good use of theme typography weights.

---

Line range hint 663-682: LGTM! Consistent tooltip styling.

The updates properly integrate with the new arrowGenerator function and maintain color consistency with the design system.

[mazursasha1990]

Requested minor changes.
The rest looks good!

Please, add a description to the PR, not just a link to the ticket.

[coderabbitai]

Actionable comments posted: 3

🧹 Outside diff range and nitpick comments (7)

src/react/src/components/FacilityListSummary.jsx (2)

Line range hint 54-60: Add missing PropTypes for 'id' and 'contributor'.

These props are used in the component but are not defined in PropTypes.

Add the following to PropTypes:

 FacilityListSummary.propTypes = {
     name: string.isRequired,
     description: string,
+    id: string.isRequired,
+    contributor: string.isRequired,
 };

---

Line range hint 5-33: Consider refactoring styles for consistency.

The component mixes frozen style objects with inline styles and contains hardcoded colors. Consider:

1. Moving all styles to the facilityListSummaryStyles object
2. Using COLOURS constants for all color values

Example refactor:

 const facilityListSummaryStyles = Object.freeze({
     nameStyles: Object.freeze({
         color: '#000 !important',
         fontWeight: 500,
         fontSize: '20px',
         margin: '8px 0',
     }),
     descriptionStyles: Object.freeze({
         color: '#000 !important',
         marginBottom: '24px',
     }),
+    containerStyles: Object.freeze({
+        alignItems: 'center',
+        display: 'flex',
+        flexDirection: 'row',
+        justifyContent: 'space-between',
+    }),
+    linkStyles: Object.freeze({
+        color: COLOURS.PRIMARY_PURPLE,
+        fontSize: '18px',
+        fontWeight: '500',
+        lineHeight: '21px',
+        textDecoration: 'none',
+    }),
 });

src/react/src/components/ConfirmActionButton.jsx (2)

Line range hint 13-18: Consider using TypeScript enum for dialog states.

The actionDialogStates object could be better represented as a TypeScript enum for improved type safety and IDE support.

Example:

enum ActionDialogStates {
    None = 'none',
    Confirm = CONFIRM_ACTION,
    Merge = MERGE_ACTION,
    Reject = REJECT_ACTION,
}

---

Line range hint 19-35: Consider breaking down the component for better maintainability.

The component has many responsibilities and props. Consider splitting it into smaller components:

• ConfirmDialog
• RejectDialog
• MergeDialog

This would:

1. Improve code organization
2. Make testing easier
3. Reduce the number of props passed to each component
4. Make the code more maintainable

src/react/src/components/Contribute/DialogTooltip.jsx (2)

7-42: Consider memoizing the arrow ref callback.

The arrow ref callback could be memoized using useCallback to prevent unnecessary re-renders.

-const DialogTooltip = ({ text, childComponent, classes }) => {
+const DialogTooltip = ({ text, childComponent, classes }) => {
+    const setArrowRefCallback = useCallback(node => {
+        setArrowRef(node);
+    }, []);

     const [arrowRef, setArrowRef] = useState(null);
     return (
         <Tooltip
             aria-label={text}
             enterDelay={200}
             leaveDelay={200}
             title={
                 <>
                     {text}
-                    <span className={classes.arrow} ref={setArrowRef} />
+                    <span className={classes.arrow} ref={setArrowRefCallback} />
                 </>
             }

---

47-55: Enhance prop types validation for classes.

The shape validation for class properties could be more specific by defining the expected CSS properties.

     classes: shape({
-        arrow: shape({}).isRequired,
-        tooltipStyles: shape({}).isRequired,
-        popperStyles: shape({}).isRequired,
-        placementLeft: shape({}).isRequired,
-        placementRight: shape({}).isRequired,
-        placementTop: shape({}).isRequired,
-        placementBottom: shape({}).isRequired,
+        arrow: shape({
+            position: string,
+            fontSize: string,
+            width: string,
+            height: string
+        }).isRequired,
+        tooltipStyles: shape({
+            fontSize: string,
+            backgroundColor: string
+        }).isRequired,
+        popperStyles: shape({}).isRequired,
+        placementLeft: shape({
+            margin: string
+        }).isRequired,
+        placementRight: shape({
+            margin: string
+        }).isRequired,
+        placementTop: shape({
+            margin: string
+        }).isRequired,
+        placementBottom: shape({
+            margin: string
+        }).isRequired,
     }).isRequired,

src/react/src/util/styles.js (1)

609-613: Enhance responsive design handling.

Consider adding more breakpoints for better responsive design support.

             [theme.breakpoints.down('md')]: {
                 justifyContent: 'initial',
                 flexDirection: 'column',
             },
+            [theme.breakpoints.down('sm')]: {
+                padding: '0 8px',
+            },

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 48e8a72 and 8bc989a.

📒 Files selected for processing (8)

• src/react/src/__tests__/components/DialogTooltip.test.js (1 hunks)
• src/react/src/__tests__/components/ProductionLocationDialog.test.js (1 hunks)
• src/react/src/components/ConfirmActionButton.jsx (2 hunks)
• src/react/src/components/Contribute/DialogTooltip.jsx (1 hunks)
• src/react/src/components/Contribute/ProductionLocationDialog.jsx (1 hunks)
• src/react/src/components/FacilityListSummary.jsx (2 hunks)
• src/react/src/util/COLOURS.js (1 hunks)
• src/react/src/util/styles.js (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• src/react/src/tests/components/ProductionLocationDialog.test.js
• src/react/src/tests/components/DialogTooltip.test.js
• src/react/src/util/COLOURS.js

🧰 Additional context used

📓 Learnings (2)

src/react/src/components/Contribute/DialogTooltip.jsx (1)

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/DialogTooltip.jsx:0-0
Timestamp: 2024-12-03T10:29:02.409Z
Learning: In the `DialogTooltip` component (`src/react/src/components/DialogTooltip.jsx`), there are existing tooltip tests that cover common cases, and further detailed tests are not necessary.

src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/Contribute/ProductionLocationDialog.jsx:33-181
Timestamp: 2024-12-10T07:09:35.641Z
Learning: In the `ProductionLocationDialog.jsx` component, hardcoded facility data is acceptable for now as per the project's requirements.

🔇 Additional comments (4)

src/react/src/components/FacilityListSummary.jsx (1)

3-3: LGTM! Good practice using color constants.

The change from hardcoded color to COLOURS.ACCENT_GREY improves maintainability.

Also applies to: 47-47

src/react/src/components/ConfirmActionButton.jsx (1)

10-10: LGTM! Good practice using color constants.

The change from hardcoded color to COLOURS.ACCENT_GREY improves maintainability.

Also applies to: 155-155

src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)

164-166: ⚠️ Potential issue

Remove console.log and implement proper handler.

Replace the console.log with proper handler implementation.

-                            onClick={() => {
-                                console.log('submit another location');
-                            }}
+                            onClick={() => {
+                                // TODO: Implement submit another location handler
+                            }}

⛔ Skipped due to learnings

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/ProductionLocationDialog.jsx:255-257
Timestamp: 2024-12-03T06:52:22.170Z
Learning: In `src/react/src/components/ProductionLocationDialog.jsx`, the 'Submit another Location' button's `onClick` handler logs to the console as the endpoint is not yet implemented on the frontend.

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/Contribute/ProductionLocationDialog.jsx:33-181
Timestamp: 2024-12-10T07:09:35.641Z
Learning: In the `ProductionLocationDialog.jsx` component, hardcoded facility data is acceptable for now as per the project's requirements.

src/react/src/util/styles.js (1)

642-643: 🛠️ Refactor suggestion

Move color value to COLOURS constant.

The backgroundColor should use a color constant from COLOURS.

         claimButton: Object.freeze({
-            backgroundColor: COLOURS.NAVIGATION,
+            backgroundColor: COLOURS.PRIMARY_BUTTON_BACKGROUND, // Add this to COLOURS
         }),

Likely invalid or redundant comment.

[sonarqubecloud]

Quality Gate passed

Issues
29 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[coderabbitai]

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)

src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)

175-176: Consistent capitalization in button labels

The button label 'Submit another Location' capitalizes 'Location'. For consistency and to adhere to common UI practices, consider using sentence case: 'Submit another location'.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 8bc989a and 986aec6.

📒 Files selected for processing (1)

• src/react/src/components/Contribute/ProductionLocationDialog.jsx (1 hunks)

🧰 Additional context used

📓 Learnings (1)

src/react/src/components/Contribute/ProductionLocationDialog.jsx (1)

Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#436
File: src/react/src/components/Contribute/ProductionLocationDialog.jsx:33-181
Timestamp: 2024-12-10T07:09:35.641Z
Learning: In the `ProductionLocationDialog.jsx` component, hardcoded facility data is acceptable for now as per the project's requirements.

🔇 Additional comments (4)

src/react/src/components/Contribute/ProductionLocationDialog.jsx (4)

1-15: Imports are correctly organized and necessary

All the imported modules and components are appropriate for the functionality of ProductionLocationDialog. The organization is clear and follows standard practices.

---

17-31: Helper functions enhance readability

The infoIcon and claimButton helper functions are well-defined and improve the readability and reusability of the code by separating concerns.

---

33-193: Component implementation is well-structured

The ProductionLocationDialog component is well-structured, utilizing Material-UI components effectively. The use of Grid for layout and custom styles enhances the user interface.

---

75-97: Use of hardcoded data is acceptable as per project requirements

The hardcoded facility data within the dialog is acceptable at this stage, according to the project's requirements.

As per the retrieved learnings, hardcoded data is acceptable for now.

[mazursasha1990]

LGTM