Skip to content

Feedback#60

Open
LaurieMV wants to merge 170 commits into
Feedbackfrom
main
Open

Feedback#60
LaurieMV wants to merge 170 commits into
Feedbackfrom
main

Conversation

@LaurieMV
Copy link
Copy Markdown

Don’t close or merge this pull request, unless you’re instructed to do so by your teacher.

  • Your teacher created this pull request as a place to leave feedback on your work. It will update automatically.
  • In this pull request, your teacher can leave comments and feedback on your code. Click the Subscribe button to be notified if that happens.
  • Click the Files changed or Commits tab to see all of the changes pushed to the default branch since the assignment started. Your teacher can see this too.

SameeraChinta13 and others added 30 commits June 3, 2025 07:41
Sameera - finished routes and added columns to item model
Cardlist grid and single page view minimum viable product
…ey to map each card; set link to url containing item.id as param; add StyledLink to styling
Mohammad-SJamal and others added 29 commits June 6, 2025 13:43
Copy link
Copy Markdown
Author

@LaurieMV LaurieMV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job, team! You completed all tiers plus lots of bonus. You did a nice job with your design and wireframe. The app has consistent styling. You provided thoughtful filter and sort. You implemented navigation using React Router. You included a shopping cart, orders, and user management. Your README is very solid and includes useful build instructions.

@@ -0,0 +1,148 @@
import React, { useContext, useState } from 'react';
import styled from "styled-components";
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great styling!

setItems(itemsData);
setFilteredItems(itemsData);
} catch (err) {
console.log("Oh no an error! ", err);
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might the end user be interested in some info about this?

<>
<h1>Inventory App</h1>
{/* Render the items */}
<AllStatesContext.Provider
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Context is a great addition

<PartFont>#{item.id}</PartFont>
</TitleAndPart>
<QuantityNumber>
<h2>${Number(item.price).toFixed(2)}</h2>
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good approach to correct money formatting. Also take a look at Intl.NumberFormat if you want to write code that is more easily translatable (localized) to other countries.

<QuantityNumber>
<h2>${Number(item.price).toFixed(2)}</h2>
<br />
<p>in stock: {item.quantity}</p>
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that you included the stock quantities, one of my favorite extensions!

Comment thread public/style.css
@@ -1,14 +1,18 @@
* {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are any of the styles of your components global, so they could go here rather than being repeated?

Comment thread server/models/Item.js
name: {
type: DataTypes.STRING,
allowNull: false,
validate: {
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ on database validation

Comment thread server/routes/items.js
if (!item) return res.status(404).json({ error: "Item not found" });

await item.destroy();
res.json({ message: "Item deleted" });
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you sure it was deleted?

Comment thread server/routes/users.js
@@ -0,0 +1,36 @@
// routes/users.js
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

++ user management

Comment thread README.md
- cd inventory-app
3. Install dependence
- npm install
- npm install react-router-dom@6.30.1
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This version is correctly included in your package and package-lock, so there should be no need to add the extra line here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants