Review 6: ProjectionGraph.jsx and ManageProjections.jsx

[beydlern]

Overview

Please pay attention too:

• Javascript issues
• React components

Review Branch

review-6

Files to review

• ProjectionGraph.jsx
• ManageProjections.jsx

Checklists

• Architecture
• Design
• JS
• React

Due date

Monday, 10/21/2024

For more information

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

[beydlern]

ProjectionGraph.jsx

• L35-58: Consolidate the PropTypes sets into one set.

ManageProjections.jsx

• L9: Incorrect comment referencing name of a different file.
• L40-54: A comment briefly describing the code would be nice for clarity.

[XavierBurt]

ProjectionGraph.jsx

I think on Line 11 {selectedChart.name} should be inside of a p tag so that we can give it a className for bootstrap or an id tag and style it how we want.

ManageProjections.jsx

Needs some more comments

It looks like the "delete" button on line 50 doesn't have any functionality, but I'm assuming we're planning on adding it later. I'm just saying it because I need more things to say about this file

[caspascual]

ProjectionGraph.jsx

ESLINT-02: Should probably start adding IDs for when we eventually make testcafes

ManageProjections.jsx

ESLINT-02: Should probably start adding IDs for when we eventually make testcafes

L17-59: Design-05: Some simple comments stating/separating what each part is would make it easier to identify each part.

[samallari]

ProjectionGraph.jsx:

• L36-43, L49-56: shape of selectedChart and dropdownData is repeated twice, is there a way to avoid this?

ManageProjections.jsx:

• L40: could add comments for readability

[jsapolu99]

ProjectionGraph.jsx

• L35-58 (Design-01, Design-08)**: The PropTypes for dropdownData and selectedChart are nearly identical, causing code duplication. To follow the DRY principle and reduce redundancy, consider consolidating the shape definitions into one reusable PropTypes.shape. This will help maintain cleaner and more manageable code.

ManageProjections.jsx -L50 (Design-01): The delete button does not yet have any functionality. This may need to be addressed by adding a TODO comment explaining that deletion logic should be implemented here in the future.

[PaytonHAH]

ProjectionGraph.jsx

• Adding component id so we can use testcafe to test this component.

ManageProjections.jsx

• Design-07: Ensure code is readable. Add comments that explains what is going on, incase modifications need to be made.

[t-tirabassi]

ProjectionGraph.jsx:

Design 01/08: The props id, name, data and then name, actual, edited, amt are repeated and could instead be refactored into two constants such as selectedChart and chartData for reusability.

ManageProjections.jsx:

JS-04: Line 20 could be destructured for readability, particularly currentUser?.profile?.companyName || 'Company Name' could be made into a constant for the profile and a constant for the companyName in case the page is expanded upon.

REACT-01: The table itself could be made into its own component for reusability and/or to simply make the code more concise.

[PaytonHAH]

Review Summary

Issues

ProjectionGraph.jsx:

1. Add testcafe id's
2. Refactor proptypes to reduce duplication

ManageProjections.jsx

1. Add comments for better readability (Low Priority)
2. Add testcafe id's
3. Delete button functionality (Already an issue in #124)
4. Possibly make table into separate component