Conversation
…css in src/index.js
Install react-bootstrap and bootstrap through npm
Install react-router, create History.js and /history route. Create Navbar.
…a and TradeLineItem
Display trade history and filter/sort
Merge database setup and login/sign up pages to main
History input
update with dashboard backup
History database integration
Settings page
API_features
Change History to Trades for clarity
Add buy/sell information to trade line, add edit/delete functionality, add responsiveness
Api feature
Auth cleanup
Update README
Fix Trades not displaying in the center
| @@ -2,6 +2,11 @@ | |||
|
|
|||
There was a problem hiding this comment.
this needs more info on how to install and run the application. Also background info on the project and planning docs
| color: white; | ||
| color: rgb(232, 222, 222); | ||
| } | ||
| /* App.css */ |
| color: #e8dede; /* Light text color */ | ||
| } | ||
|
|
||
| /* index.css */ |
There was a problem hiding this comment.
This comment does not make sense to me somehow 🤔
| } | ||
|
|
||
| /* Additional dark mode styles */ | ||
| /* App.css */ |
There was a problem hiding this comment.
Generally, if you mean to make sections for different files in this css file, it might make sense to create css files for specific files instead of using a single css file for the whole app and creating sections
| import Row from "react-bootstrap/Row"; | ||
| import Col from "react-bootstrap/Col"; | ||
|
|
||
| const Accountsummary = () => { |
There was a problem hiding this comment.
File name and component name should use PascalCase. AccountSummary
|
|
||
| useEffect(() => { | ||
| // Call the API for each company | ||
| getAlphaVantageData("AMZN"); |
There was a problem hiding this comment.
I think it would be better to have an array of symbols here, and then run a loop on them with this function instead of manually calling the function multiple times in a row
| {/* <img src={logo} className="App-logo" alt="logo" /> */} | ||
| {/* <p> | ||
| Edit <code>src/App.js</code> and save to reload. | ||
| </p> */} | ||
| {/* <Educationalmaterial /> */} |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [tradesArr, sort]); | ||
|
|
||
| const sortTrades = (arr, sortMethod) => { |
There was a problem hiding this comment.
I would include the way trades are sorted in the function name. ascending, descending, alphabetical, numerical etc.
| }; | ||
|
|
||
| const filterTrades = (filter) => { | ||
| const loadedTrades = []; |
There was a problem hiding this comment.
this could be moved within the else block, as loadedTrades is only used in there
| let newTradesArr = tradesArr; | ||
| for (let i = 0; i < newTradesArr.length; i++) { | ||
| if (newTradesArr[i].key === tradeUid) { | ||
| newTradesArr.splice(i, 1); |
There was a problem hiding this comment.
| newTradesArr.splice(i, 1); | |
| const newTradesArr = [...tradesArr].splice(i, 1); |
There was a problem hiding this comment.
The way you copied newTradesArr from tradesArr, you will actuall perform the splice on the original array as well
| // import { Link } from "react-router-dom"; | ||
| // import Container from "react-bootstrap/Container"; | ||
|
|
||
| const Educationalmaterial = () => { |
| <Link className="nav-link text-white" to="/"> | ||
| Home | ||
| </Link> | ||
| <Link className="nav-link text-white" to="/dash"> | ||
| Dashboard | ||
| </Link> | ||
| <Link className="nav-link text-white" to="/trades"> | ||
| Trades | ||
| </Link> | ||
| <Link className="nav-link text-white" to="/insights"> | ||
| Insights | ||
| </Link> | ||
| <Link className="nav-link text-white" to="/settings"> | ||
| Settings | ||
| </Link> |
There was a problem hiding this comment.
I think you could create a reusable component for this and render from a map instead of hardcoding it like here
4 participants
group members: