search

Team-Half-and-Half / spirebooks

MIT License
0 stars 0 forks source link

Review 8: BudgetPLCollection.js and Footer.jsx #142

Closed bksnelson closed 1 week ago

bksnelson commented 2 weeks ago

Overview

Please pay attention too:

Review Branch

review-8

Files to review

Checklists

Due date

Monday, 11/4/2024

For more information

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

t-tirabassi commented 2 weeks ago

BudgetPLCollection.js:

JS-03: The spread operator can be used for updateData on line 48 instead of using the const.

JS-05: Arrow functions can be used for subscribeBudgetPL on line 93 and subscribeBudgetPLAdmin on line 104.

Footer.jsx:

Design-01/08: The different sections of the footer could be refactored into a separate const and then just mapped for each column in Footer to avoid repetition.

REACT-01: On top of refactoring, the column layout and props could be set up in a subcomponent and then just called into the Footer component.

beydlern commented 1 week ago

BudgetPLCollection.js

The define, update, and removeIt functions should incorporate error handling.

Footer.jsx

Each column section of the footer element contains duplicated code, the hyperlink tags and the line break tags, that should be restructured.

caspascual commented 1 week ago

BudgetPLCollection.js

L145: REACT-03: Should this instead be a default export with the same name?

Footer.jsx

ESLINT-02: Should add an ID for future TestCafe

PaytonHAH commented 1 week ago

BudgetPLCollection.js

  1. JS-05: Use arrow functions when appropriate. Functions in this file can be converted to arrow functions.

Footer.jsx

  1. TestCafe ID's Required
  2. Design-08: Ensure code is DRY, some duplication with the columns.
jsapolu99 commented 1 week ago

BudgetPLCollection.js: Use arrow functions where appropriate, such as in subscribeBudgetPL and subscribeBudgetPLAdmin, and consider adding error handling to define, update, and removeIt for better robustness.

Footer.jsx: Refactor repeated column elements into a separate constant and map them to each footer column to enhance readability and keep the code DRY.

samallari commented 1 week ago

BudgetPLCollection.js:

Footer.jsx:

bksnelson commented 1 week ago

BudgetPLCollection.js Publication method could be split based on role. More roles will be added, making the method larger than needed. But would require changing base collection.

Footer.jsx Some additional padding on y axis could be added for container. L12

samallari commented 1 week ago

Overview of Review Meeting

New Issues: