Review 9 : DisplayAudBalSheet.jsx and ImageCarousel.jsx

[Sydnee-You]

Overview

The focus for this code review will be centered around the DisplayAudBalSheet.jsx and ImageCarousel.jsx page.
Please pay attention too:

• Javascript issues
• React components

Review Branch

review-9

Files to review

• DisplayAudBalSheet.jsx
• ImageCarousel.jsx

Checklists

• Architecture
• Design
• JS
• React

Due date

11/11/24

For more information

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

[Sydnee-You]

DisplayAudBalSheet.jsx

DE-01 lines 104 to 521 look pretty similar. Could maybe make a separate component just for that layout.

ImageCarousel.jsx

DE-03 on lines 26, 36, 48 (can't resolve directory images and cannot resolve a specific file),

[blakewatanabe]

DisplayAudBalSheet.jsx

DE-07: Ensure code is DRY - There is some repetition with the inline style css code like padding top and height. Maybe it could be moved to the style css file to removed some of the repetition. DE-06: Ensure code is readable - The code is readable with no dead code or errors.

ImageCarousel.jsx

DE-07: Ensure code is DRY - There is some repetition with the inline style css code (like background color, border color, width). Maybe it could be moved to the style css file to removed some of the repetition. DE-06: Ensure code is readable - The rest of the code is very readable with no errors.

[luisy-eng]

DisplayAudBalSheet.jsx

• DE-07: Ensure code is DRY - Inline CSS styles such as padding-top and height are repeated in multiple places. Consider moving these styles to an external CSS file to reduce redundancy.
• DE-06: Ensure code is readable - The code is clean and readable, with no unused or erroneous code present.

ImageCarousel.jsx

• DE-07: Ensure code is DRY - Repeated inline styles, including background-color, border-color, and width, could be extracted into a CSS file to eliminate duplication.
• DE-06: Ensure code is readable - The rest of the code is well-structured and free of errors, maintaining readability.

[cash-baker]

DisplayAudBalSheet.jsx

• DE-07 (Ensure code is dry): Sections of code are repeated throughout the file (lines 99-500). Moving styles to the external style.css file and refactoring repeated code would cut down on redundant code.

ImageCarousel.jsx

• DE-07 (Ensure code is dry): Repeated code with identical styling and class types. Move styles to external file, refactor repeat code https://github.com/manoa-inspire/MATP/blob/79bda158bf2f2855f3262748d82098619b469855/app/imports/ui/components/ImageCarousel.jsx#L87-L110

[TannerBerry]

DisplayAudBalSheet.jsx JS-06 (ln 44): Inline styling for Row components should be moved to CSS or SCSS files for a cleaner separation of concerns. JS-07 (ln 539): The submit function for the could benefit from adding validation checks before submitting to enhance data integrity and user feedback. ImageCarousel.jsx: JS-05 (ln 27, 28): Inline styles should be moved to a CSS/SCSS file for better separation of concerns. JS-09 (ln 33): the img tag should be placed in CSS to improve maintainability and reduce inline styling. JS-09 (ln 61): Repeated button can be moved to a CSS class for reuse across buttons. JS-07 (ln 81): Repeated usage should be extracted into a shared class in CSS/SCSS for improved readability and maintainability.

[cjochim]

DisplayAudBalSheet.jsx: There are repeated CSS styles that could be moved to CSS file to keep code DRY. Code is readable and there's no deeply nested conditionals.

ImageCarousel.jsx: Images need to be replaced since the page is updated. There's repeated CSS styles that could be refactored and reused in the CSS file.

[RomaMalasarte]

DisplayAudBalSheet.jsx

L101-downwards: DE-06, Avoid inline styles, move them to the CSS file to maintain readability

L107,L112, L117, and similar lines: DE-07, Use mapping functions and reduce redundancy.

ImageCarousel.jsx L19, 28, 38, 82, and those similar: DE-06, Avoid inline styles and make code readable. Move to CSS file