Review 3: UserSettings.jsx and CustomLineChart.jsx

[beydlern]

Overview

Please pay attention too:

• Javascript issues
• React components

Review Branch

review-3

Files to review

• UserSettings.jsx
• CustomLineChart.jsx

Checklists

• Architecture
• Design
• JS
• React

Due date

Monday, 9/30/2024

For more information

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

[t-tirabassi]

UserSettings.jsx:

Design-05: The comments on lines 26 and 46 can probably be removed as they serve no purpose.

Design-07: The code from lines 25-55 and lines 60-77 could maybe be spaced out for readability.

React-01: Lines 11-21 could possibly be made into a subcomponent.  

CustomChartLine.jsx:

Design-01: The stylization from lines 53-57 for LineChart could be made into a CSS class. The type and stroke from lines 65 and 66 could also be made into a CSS class if possible.

[beydlern]

UserSettings.jsx

DESIGN-05: L:35 comment disables an eslint check on the following line, which is bad practice.

DESIGN-07: Parameters of submit function (L:27) should be commented on to describe them.

CustomLineChart.jsx

DESIGN-01: Refactor styling into a CSS class or id (L52-59) and (L65-66)

[jsapolu99]

UserSettings.jsx DESIGN-01: L:33,38 There is duplication in the error and success handling for the two Meteor.call methods (for updating company name and password). Refactoring this into a reusable function could reduce the repeated code for displaying swal alerts.

CustomLineChart.jsx Design-01: L 9–11 and Line 15–17: The structure of the data objects repeats across multiple entries.

[caspascual]

UserSettings.jsx

L35: ESLINT-01: Remove this comment that disables eslint, then rename the second error differently

CustomLineChart.jsx

L6-48: Design-09: Example data should be placed in settings.development.json

L51-68: Design-09: Styling could be placed into style.css

[PaytonHAH]

UserSettings.jsx Design-05: Irrelevant Comment for code (Line 35)

Custom LineChart.jsx Design-09: Ensure reusable code is exported or made available in the appropriate place: Default Data for graphs should be in the json default file. Design-07: Ensure code is readable: Possibly add comments for better readability incase modification is required.

[samallari]

CustomLineChart.jsx

• adding a data parameter to the component would allow the code to accept different values for the line chart

[bksnelson]

UserSettings.jsx DS-02: Multiple If/else. Might be another way to handle this. We should verify that this is a safe way to change passwords. Might be good to include useTracker for updates. Currently able to change password with same older password.

CustomLineChart.jsx DS-05: Could use comments in other sections.

[XavierBurt]

UserSettings.jsx I think there might be a problem with calling Meteor.call inside of Meteor.call (Besides just an eslint problem), I'm assuming that it creates a child process, which might have undefined behavior if a user spams the button.

CustomLineChart.jsx

I think the data starting from line 6 and ending at line 49 should be in settings.development.json

[beydlern]

Overview of Review Meeting

New issues:

• cleanup UserSettings.jsx eslint comment by removing Meteor.call() in if-else
• UserSettings.jsx implement a safe way to change passwords
• cleanup CustomLineChart.jsx by refactoring the styling into classes or ids in a CSS file
• Default data should be in settings.development.json in CustomLineChart.jsx