Review: ContactUs.jsx and UserPage.jsx

[blakewatanabe]

Overview

The focus for this code review will be centered around the ContactUs page and User page.

Please pay attention too:

• Javascript issues
• React components

Review Branch

review-3

Files to review

• ContactUs.jsx
• UserPage.jsx

Checklists

• Architecture
• Design
• JS
• React

Due date

09/30/24 by 9:00am

For more information

The review process is documented at: http://courses.ics.hawaii.edu/ics414s21/morea/review/reading-idpm-review.html

[cjochim]

ContactUs.jsx

• DE-05: Adding more comments for the purpose of handleSubmit and Meteor.call
• could use arrow function for handleSubmit
• code is readable and organized

UserPage.jsx

• DE-01: duplication in dataset prep — create a helper function to keep the code DRY
• meaningful variable names

[Sydnee-You]

ContactUs.jsx AR-02: Contact Us is a page so shouldn't it be in the pages directory instead of the components directory?

UserPage.jsx Nothing in particular

[blakewatanabe]

ContactUs.jsx

https://github.com/manoa-inspire/MATP/blob/7f56f3652bbdff83a792884ca060823540681bbc/app/imports/ui/components/ContactUs.jsx#L10 JS-03: You could refactor the form values for better readability

UserPage.jsx

DE-01: Can you refactor - The component is quite large and can benefit from some refactoring. You could split the table, charts, and the button into smaller, reusable components.

DE-04 & DE-05: Eliminate large comment blocks and ensure comments are appropriate - Comments are minimal and appropriate, but there are none for key sections like mockLogs or the chart configurations, which may benefit from some.

[TannerBerry]

UserPage.jsx AR-01 (ln N/A): The file name UserPage.jsx does not follow the lowercase file naming convention. It should be renamed to userPage.jsx. DE-06 (ln N/A): Code is generally readable, but the repetitive definitions of revenueData, expensesData, and netProfitData could be refactored into a reusable structure if similar data setups are needed elsewhere. RE-01 (ln 67, ln 144, ln 150): Consider breaking down the table and chart sections into separate components for better modularity and maintainability, such as a DataTable component for the table and a ChartContainer for the charts. ContactUs.jsx AR-01 (ln N/A): The file name ContactUs.jsx does not follow the lowercase naming convention. It should be renamed to contactUs.jsx. ES-01 (ln 33, ln 39, ln 45, ln 51): ESLint disable comments for label accessibility (eslint-disable-next-line jsx-a11y/label-has-associated-control) are present. It's recommended to resolve these issues by ensuring that labels have associated controls rather than disabling the rule

[luisy-eng]

Code Review for UserPage.jsx:

Architecture Checklist:

• AR-01 (File Naming Conventions):
  • Follows camelCase (UserPage.jsx).

Design Checklist:

• DE-01 (Refactor to Reduce Duplication):

  • The dataset creation for revenueData, expensesData, and netProfitData are similar. Consider refactoring into a reusable function to reduce duplication.

• DE-02 (Avoid Nested Conditionals):

  • No deeply nested conditionals observed.

• DE-03 (Eliminate Dead Code):

  • Review if all the mock data is required. Mock data might be trimmed down if unnecessary for production.

• DE-05 (Ensure Comments are Appropriate):

  • No excessive comments. The comments presented are clear and concise.

• DE-06 (Ensure Code is Readable):

  • Code readability is generally good. Could benefit from refactoring similar blocks (as noted above).

JavaScript and ESLint Checklist:

• JS-01 (Name Constructs Appropriately):

  • Variables are named clearly (e.g., dates, revenues, expenses).

• JS-02 (Use the Spread Operator):

  • In places where arrays are being manipulated, ensure the spread operator is used if applicable.

• JS-03 (Use Object Destructuring):

  • Object destructuring is used appropriately.

• JS-06 (Avoid console.logs):

  • No console.log statements were observed.

React Checklist:

• RE-01 (Components Should Be Simple):

  • Consider breaking down this large component if it does too many things (e.g., separating chart rendering into smaller subcomponents).

• RE-04 (Destructure Props):

  • If props are passed, consider destructuring for clarity (though this specific file does not seem to rely on external props).

---

Code Review for ContactUs.jsx:

Architecture Checklist:

• AR-01 (File Naming Conventions):
  • ContactUs.jsx follows proper camelCase.

Design Checklist:

• DE-01 (Refactor to Reduce Duplication):

  • No noticeable code duplication.

• DE-02 (Avoid Nested Conditionals):

  • No deeply nested conditionals present.

• DE-03 (Eliminate Dead Code):

  • No dead code observed.

• DE-06 (Ensure Code is Readable):

  • The component is straightforward and readable.

JavaScript and ESLint Checklist:

• JS-01 (Name Constructs Appropriately):

  • Appropriate naming conventions are followed (e.g., handleSubmit, form-control).

• JS-03 (Use Object Destructuring):

  • Could consider destructuring form data, though currently the form is simple and doesn’t require much.

React Checklist:

• RE-01 (Components Should Be Simple):

  • The ContactUs component is simple and well-organized.

• RE-04 (Destructure Props):

  • No props are being passed here, so this doesn't apply.

Next Steps/Improvements:

1. UserPage.jsx:

   • Refactor similar blocks of code into a helper function for dataset creation.
   • Ensure proper use of the spread operator if modifying arrays.
   • If necessary, break down the component into smaller pieces for easier readability and reusability.

2. ContactUs.jsx:

   • Consider destructuring form input data for cleaner code.

[RomaMalasarte]

ContactUs.jsx L10-13: JS-04, Use object deconstruction

UserPage.jsx L176, L183, L187: Design-01, Code has repeated structures, refactor the code into reusable component to mitigate duplication.

L152: JS-04, Use object deconstruction to make log object more concise.

[juncell-venzon]

UserPage.jsx

DE-03: Add ARIA labels to buttons and tables to ensure better accessibility, such as labeling the "Add New Data" button more descriptively.

DE-07 : Add a loading indicator for the chart data or table in case the data is being fetched from an API. This will enhance the user experience when the data takes time to load.

DE-01 : The code for revenueData, expensesData, and netProfitData is repetitive. Create a function to generate datasets thus reducing duplication,

ContactUs.jsx

DE-07: Add a loading indicator for the chart data or table in case the data is being fetched from an API. This will alloww the user to be able to know when the data takes time to load.

JS-01 : Ensure consistency in naming conventions.

[cash-baker]

UserPage.jsx

• (DE-06) Readability is good, but consider refactoring certain code blocks to improve readability
• (JS-03) Would also consider refactoring the form values, as Blake mentioned

ContactUs.jsx

• (DE-01) Refactor any reused code fragments
• (DE-04) Eliminate large comment blocks