Skip to content
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

002 sep 20 vendor view design #20

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
372 changes: 336 additions & 36 deletions Clients/package-lock.json

Large diffs are not rendered by default.

5 changes: 4 additions & 1 deletion Clients/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
"@emotion/styled": "^11.13.0",
"@fontsource/roboto": "^5.1.0",
"@mui/icons-material": "^6.1.0",
"@mui/material": "^6.1.0",
"@mui/lab": "^6.0.0-beta.10",
"@mui/material": "^6.1.1",
"@mui/styled-engine-sc": "^6.1.0",
"@mui/x-date-pickers": "^7.18.0",
"@reduxjs/toolkit": "^2.2.7",
"@svgr/rollup": "^8.1.0",
"dayjs": "^1.11.13",
"react": "^18.3.1",
"react-dom": "^18.3.1",
"react-redux": "^9.1.2",
Expand Down
3 changes: 3 additions & 0 deletions Clients/src/presentation/assets/icons/close.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
3 changes: 3 additions & 0 deletions Clients/src/presentation/assets/icons/left-arrow-long.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
89 changes: 89 additions & 0 deletions Clients/src/presentation/components/IconButton/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
import {
Menu,
MenuItem,
IconButton as MuiIconButton,
useTheme,
} from "@mui/material";
import { ReactComponent as Setting } from "../../assets/icons/setting.svg";
import { useState } from "react";
import BasicModal from "../Modals/Basic";
Comment on lines +1 to +9
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider using absolute imports for better maintainability.

The import for the SVG icon uses a relative path:

import { ReactComponent as Setting } from "../../assets/icons/setting.svg";

Consider using absolute imports to make the code more resilient to file structure changes and easier to maintain. This can be achieved by configuring path aliases in your TypeScript configuration.


const IconButton = () => {
const theme = useTheme();
const [anchorEl, setAnchorEl] = useState(null);
const [actions, setActions] = useState({});
const [isOpen, setIsOpen] = useState(false);
Comment on lines +11 to +15
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance type safety with TypeScript interfaces.

Consider defining an interface for the actions state to improve type safety and code readability:

interface Actions {
  id: string;
  url: string;
}

const [actions, setActions] = useState<Actions>({} as Actions);

This will provide better type checking and autocompletion when working with the actions state.


const openMenu = (event: any, id: any, url: any) => {
console.log("open menu");
event.preventDefault();
event.stopPropagation();
setAnchorEl(event.currentTarget);
setActions({ id: id, url: url });
console.log(actions);
};
Comment on lines +17 to +24
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve type safety and remove console.log statements.

  1. Replace any types with more specific types:
const openMenu = (event: React.MouseEvent, id: string, url: string) => {
  // ...
}
  1. Remove or comment out console.log statements before pushing to production:
// console.log("open menu");
// console.log(actions);

These changes will enhance type safety and prepare the code for production deployment.


const openRemove = (e: any) => {
closeMenu(e);
setIsOpen(true);
};

const closeMenu = (e: any) => {
e.stopPropagation();
setAnchorEl(null);
};
Comment on lines +26 to +34
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider optimizing event handling.

The closeMenu function is called within openRemove, but both stop event propagation. This might be redundant. Consider refactoring to:

const openRemove = () => {
  setAnchorEl(null);
  setIsOpen(true);
};

const closeMenu = () => {
  setAnchorEl(null);
};

Then, update the onClick handler in the MenuItem to:

onClick={(e) => {
  e.stopPropagation();
  openRemove();
}}

This approach reduces redundancy and separates concerns more clearly.


return (
<>
<MuiIconButton
sx={{
"&:focus": {
outline: "none",
},
"& svg path": {
stroke: theme.palette.other.icon,
},
}}
onClick={(event) => {
event.stopPropagation();
openMenu(event, "someId", "someUrl");
}}
>
<Setting />
</MuiIconButton>

<Menu
anchorEl={anchorEl}
open={Boolean(anchorEl)}
onClose={(e) => closeMenu(e)}
slotProps={{
paper: {
sx: {
width: 190,
"& ul": { p: theme.spacing(2.5) },
"& li": { m: 0, fontSize: 13 },
"& li:hover": { borderRadius: 4 },
"& li:last-of-type": {
color: theme.palette.error.main,
},
boxShadow: theme.boxShadow,
},
},
}}
>
<MenuItem>Edit</MenuItem>
<MenuItem
onClick={(e) => {
e.stopPropagation();
openRemove(e);
}}
>
Remove
</MenuItem>
Comment on lines +74 to +82
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enhance accessibility and user experience.

Consider adding aria labels and keyboard navigation support to improve accessibility:

<MenuItem aria-label="Edit vendor">Edit</MenuItem>
<MenuItem
  aria-label="Remove vendor"
  onClick={(e) => {
    e.stopPropagation();
    openRemove(e);
  }}
  onKeyDown={(e) => {
    if (e.key === 'Enter' || e.key === ' ') {
      e.preventDefault();
      openRemove(e);
    }
  }}
>
  Remove
</MenuItem>

This change will make the component more accessible to users relying on screen readers or keyboard navigation.

</Menu>
<BasicModal isOpen={isOpen} setIsOpen={() => setIsOpen(false)} />
</>
);
Comment on lines +36 to +86
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Refactor render method for improved maintainability and readability.

  1. Extract inline styles to a separate styles object:
const styles = {
  iconButton: {
    "&:focus": {
      outline: "none",
    },
    "& svg path": {
      stroke: theme.palette.other.icon,
    },
  },
  menu: {
    width: 190,
    "& ul": { p: theme.spacing(2.5) },
    "& li": { m: 0, fontSize: 13 },
    "& li:hover": { borderRadius: 4 },
    "& li:last-of-type": {
      color: theme.palette.error.main,
    },
    boxShadow: theme.boxShadow,
  },
};
  1. Simplify the Menu component by using the extracted styles:
<Menu
  anchorEl={anchorEl}
  open={Boolean(anchorEl)}
  onClose={closeMenu}
  PaperProps={{ sx: styles.menu }}
>
  {/* Menu items */}
</Menu>
  1. Consider extracting the Menu and Modal into separate components for better separation of concerns.

These changes will improve the overall structure and maintainability of the component.

};

export default IconButton;
16 changes: 16 additions & 0 deletions Clients/src/presentation/components/Inputs/Datepicker/index.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
.mui-date-picker {
height: 34px;
}

.mui-date-picker .MuiInputBase-root {
height: 34px;
}

.mui-date-picker .MuiInputBase-root input {
padding: 5px;
font-size: 13px;
}

.mui-date-picker > .MuiFormLabel-root {
font-size: 13px;
}
81 changes: 81 additions & 0 deletions Clients/src/presentation/components/Inputs/Datepicker/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { Stack, SxProps, Theme, Typography, useTheme } from "@mui/material";
import { AdapterDayjs } from "@mui/x-date-pickers/AdapterDayjs";
import { LocalizationProvider } from "@mui/x-date-pickers/LocalizationProvider";
import { DatePicker as MuiDatePicker } from "@mui/x-date-pickers/DatePicker";
import "./index.css";
Comment on lines +1 to +5
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

CSS Import Can Be Refactored Into MUI's Styling System

The index.css file contains minimal styling that can be integrated directly into the component using MUI's sx prop or styled components. Consider removing the CSS import and applying these styles within the component for better maintainability.

Analysis chain

Verify the necessity of the CSS import.

The component imports a CSS file on line 5. Ensure that this file contains necessary styles that can't be achieved using MUI's styling system. If possible, consider integrating these styles into the component using MUI's sx prop or styled components for better maintainability.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check the contents of the imported CSS file.

# Test: Display the contents of the CSS file
cat Clients/src/presentation/components/Inputs/Datepicker/index.css

Length of output: 307


interface DatePickerProps {
label?: string;
isRequired?: boolean;
isOptional?: boolean;
optionalLabel?: string;
sx?: SxProps<Theme> | undefined;
}

const DatePicker = ({
label,
isRequired,
isOptional,
optionalLabel,
sx,
}: DatePickerProps) => {
const theme = useTheme();
return (
<Stack
sx={{
"& fieldset": {
borderColor: theme.palette.border,
borderRadius: theme.shape.borderRadius,
},
"&:not(:has(.Mui-disabled)):not(:has(.input-error)) .MuiOutlinedInput-root:hover:not(:has(input:focus)):not(:has(textarea:focus)) fieldset":
{
borderColor: theme.palette.border,
},
"&:has(.input-error) .MuiOutlinedInput-root fieldset": {
borderColor: theme.palette.error.text,
},
}}
>
Comment on lines +24 to +38
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider extracting complex styling logic.

The styling logic in the Stack component is quite complex. Consider extracting this into a separate styled component or a custom hook for better readability and maintainability.

Here's a suggested refactor:

const useCustomDatePickerStyles = (theme: Theme) => ({
  "& fieldset": {
    borderColor: theme.palette.border,
    borderRadius: theme.shape.borderRadius,
  },
  "&:not(:has(.Mui-disabled)):not(:has(.input-error)) .MuiOutlinedInput-root:hover:not(:has(input:focus)):not(:has(textarea:focus)) fieldset":
    {
      borderColor: theme.palette.border,
    },
  "&:has(.input-error) .MuiOutlinedInput-root fieldset": {
    borderColor: theme.palette.error.text,
  },
});

// In the component:
const customStyles = useCustomDatePickerStyles(theme);
return (
  <Stack sx={customStyles}>
    {/* ... */}
  </Stack>
);

{label && (
<Typography
component="h3"
color={theme.palette.text.secondary}
fontSize={13}
fontWeight={500}
marginBottom={theme.spacing(3)}
>
{label}
{isRequired ? (
<Typography
component="span"
ml={theme.spacing(1)}
color={theme.palette.error.text}
>
*
</Typography>
) : (
""
)}
{isOptional ? (
<Typography
component="span"
fontSize="inherit"
fontWeight={400}
ml={theme.spacing(2)}
sx={{ opacity: 0.6 }}
>
{optionalLabel || "(optional)"}
</Typography>
) : (
""
)}
</Typography>
)}
Comment on lines +39 to +73
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Simplify label rendering logic.

The label rendering logic is repetitive and can be simplified for better readability.

Consider refactoring the label rendering as follows:

const renderLabel = () => {
  if (!label) return null;
  
  return (
    <Typography
      component="h3"
      color={theme.palette.text.secondary}
      fontSize={13}
      fontWeight={500}
      marginBottom={theme.spacing(3)}
    >
      {label}
      {isRequired && (
        <Typography
          component="span"
          ml={theme.spacing(1)}
          color={theme.palette.error.text}
        >
          *
        </Typography>
      )}
      {isOptional && (
        <Typography
          component="span"
          fontSize="inherit"
          fontWeight={400}
          ml={theme.spacing(2)}
          sx={{ opacity: 0.6 }}
        >
          {optionalLabel || "(optional)"}
        </Typography>
      )}
    </Typography>
  );
};

// In the component's return statement:
return (
  <Stack sx={...}>
    {renderLabel()}
    {/* ... */}
  </Stack>
);

<LocalizationProvider dateAdapter={AdapterDayjs}>
<MuiDatePicker className="mui-date-picker" sx={sx} />
</LocalizationProvider>
Comment on lines +74 to +76
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Utilize component props in MuiDatePicker.

The MuiDatePicker component doesn't seem to utilize all the props passed to the DatePicker component. Consider passing relevant props to enhance its functionality.

Update the MuiDatePicker usage as follows:

<MuiDatePicker 
  className="mui-date-picker" 
  sx={sx}
  label={label}
  required={isRequired}
  // Add any other relevant props from DatePickerProps
/>

</Stack>
);
};

export default DatePicker;
3 changes: 3 additions & 0 deletions Clients/src/presentation/components/Inputs/Field/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ interface FieldProps {
onInput?: (event: React.FormEvent<HTMLInputElement>) => void;
error?: string;
disabled?: boolean;
width?: number | string;
}

const Field = forwardRef(
Expand All @@ -71,6 +72,7 @@ const Field = forwardRef(
onInput,
error,
disabled,
width,
}: FieldProps,
ref: ForwardedRef<HTMLInputElement>
) => {
Expand All @@ -94,6 +96,7 @@ const Field = forwardRef(
"&:has(.input-error) .MuiOutlinedInput-root fieldset": {
borderColor: theme.palette.error.text,
},
width: width,
}}
>
{label && (
Expand Down
112 changes: 112 additions & 0 deletions Clients/src/presentation/components/Modals/Basic/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
/**
* A modal component that displays a confirmation dialog for deleting a vendor.
*
* @component
* @param {BasicModalProps} props - The properties for the BasicModal component.
* @param {boolean} props.isOpen - A boolean indicating whether the modal is open.
* @param {function} props.setIsOpen - A function to set the modal's open state.
*
* @returns {JSX.Element} The rendered modal component.
*
* @example
* <BasicModal isOpen={isOpen} setIsOpen={setIsOpen} />
*/

import { Button, Modal, Stack, Typography, useTheme } from "@mui/material";

interface BasicModalProps {
isOpen: boolean;
setIsOpen: (isOpen: boolean) => void;
}

const BasicModal: React.FC<BasicModalProps> = ({ isOpen, setIsOpen }) => {
const theme = useTheme();
return (
<Modal open={isOpen} onClose={setIsOpen}>
<Stack
gap={theme.spacing(2)}
color={theme.palette.text.secondary}
sx={{
position: "absolute",
top: "50%",
left: "50%",
transform: "translate(-50%, -50%)",
width: 450,
bgcolor: theme.palette.background.main,
border: 1,
borderColor: theme.palette.border,
borderRadius: theme.shape.borderRadius,
boxShadow: 24,
p: theme.spacing(15),
"&:focus": {
outline: "none",
},
}}
>
<Typography id="modal-delete-vendor" fontSize={16} fontWeight={600}>
Delete this vendor?
</Typography>
<Typography
id="delete-monitor-confirmation"
fontSize={13}
textAlign={"justify"}
>
When you delete this vendor, all data related to this vendor will be
removed. This action is non-recoverable.
</Typography>
<Stack
direction="row"
gap={theme.spacing(4)}
mt={theme.spacing(12)}
justifyContent="flex-end"
>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="text"
color="inherit"
onClick={(e) => {
e.stopPropagation();
setIsOpen(false);
}}
sx={{
width: 100,
textTransform: "capitalize",
fontSize: 13,
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
backgroundColor: "transparent",
},
}}
>
Cancel
</Button>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="contained"
color="error"
sx={{
width: 140,
textTransform: "capitalize",
fontSize: 13,
backgroundColor: theme.palette.error.main,
boxShadow: "none",
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
},
}}
>
Delete vendor
</Button>
</Stack>
</Stack>
</Modal>
);
Comment on lines +57 to +109
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Button styling is good, but deletion logic is missing.

The button styling is consistent with the overall design, and the Cancel button's functionality is correctly implemented.

The "Delete vendor" button lacks an onClick handler to perform the actual deletion. Consider adding the deletion logic or a placeholder for it:

<Button
  disableRipple
  disableFocusRipple
  disableTouchRipple
  variant="contained"
  color="error"
+ onClick={(e) => {
+   e.stopPropagation();
+   // TODO: Implement vendor deletion logic
+   setIsOpen(false);
+ }}
  sx={{
    // ... (existing styles)
  }}
>
  Delete vendor
</Button>
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<Stack
direction="row"
gap={theme.spacing(4)}
mt={theme.spacing(12)}
justifyContent="flex-end"
>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="text"
color="inherit"
onClick={(e) => {
e.stopPropagation();
setIsOpen(false);
}}
sx={{
width: 100,
textTransform: "capitalize",
fontSize: 13,
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
backgroundColor: "transparent",
},
}}
>
Cancel
</Button>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="contained"
color="error"
sx={{
width: 140,
textTransform: "capitalize",
fontSize: 13,
backgroundColor: theme.palette.error.main,
boxShadow: "none",
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
},
}}
>
Delete vendor
</Button>
</Stack>
</Stack>
</Modal>
);
<Stack
direction="row"
gap={theme.spacing(4)}
mt={theme.spacing(12)}
justifyContent="flex-end"
>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="text"
color="inherit"
onClick={(e) => {
e.stopPropagation();
setIsOpen(false);
}}
sx={{
width: 100,
textTransform: "capitalize",
fontSize: 13,
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
backgroundColor: "transparent",
},
}}
>
Cancel
</Button>
<Button
disableRipple
disableFocusRipple
disableTouchRipple
variant="contained"
color="error"
onClick={(e) => {
e.stopPropagation();
// TODO: Implement vendor deletion logic
setIsOpen(false);
}}
sx={{
width: 140,
textTransform: "capitalize",
fontSize: 13,
backgroundColor: theme.palette.error.main,
boxShadow: "none",
borderRadius: "4px",
"&:hover": {
boxShadow: "none",
},
}}
>
Delete vendor
</Button>
</Stack>
</Stack>
</Modal>
);

};

export default BasicModal;
Loading