Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: added a stock tracker component in kitchen-sink package #667

Merged
merged 9 commits into from Oct 6, 2023

Conversation

Abhishek-Mallick
Copy link
Contributor

@Abhishek-Mallick Abhishek-Mallick commented Oct 5, 2023

Please describe the changes this PR makes and why it should be merged:
This PR aims to demonstrate a stock tracker component using million incorporated with a alphavantage api
Status
Closes : #611

2023-10-05.14-06-58.mp4

Semantic versioning classification:

  • This PR changes the codebase
    • This PR includes breaking changes (methods removed or renamed, parameters moved or removed)
    • This PR changes the internal workings with no modifications to the external API (bug fixes, performance improvements)
  • This PR only includes non-code changes, like changes to documentation, README, etc.

@vercel
Copy link

vercel bot commented Oct 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
million-kitchen-sink ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 5, 2023 3:53pm
sink ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 5, 2023 3:53pm

@Timonwa
Copy link
Contributor

Timonwa commented Oct 5, 2023

Hi @Abhishek-Mallick, thank you for your PR. This looks good, but I have only two concerns.

  1. You didn't use Million.js in your project, which is required.

  2. Not everyone knows about stocks or the 'symbol' for company stocks. So, it would be hard for one to understand what exactly they are supposed to input or search for. When I searched for Google, I got NAN.

Can you add a sentence that tells the user what a valid input is? You could also list some examples of stocks they can search for. And if the API also returns the company's name or any other info, can you display them to the user?

You could even have a dropdown list of stocks or companies they can select, which is a more user-friendly approach than guessing the stock name or going to google to search for it.

@Abhishek-Mallick
Copy link
Contributor Author

@Timonwa @tobySolutions
Necessary Changes have been made

  • Million incorporated
  • Example table added for company and its associated ticker
  • Company information like description , market cap, dividend-per-share etc fetched from API
  • Styling improvements
2023-10-05.21-16-42.mp4

@tobySolutions
Copy link
Contributor

Thank you very much for the review @Timonwa. Good job @Abhishek-Mallick; I'm reviewing now.

Copy link
Contributor

@tobySolutions tobySolutions left a comment

Choose a reason for hiding this comment

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

LGTM!!

@tobySolutions tobySolutions merged commit 394f626 into aidenybai:main Oct 6, 2023
6 checks passed
@Timonwa
Copy link
Contributor

Timonwa commented Oct 8, 2023

This looks so much better @Abhishek-Mallick. Awesome job. 👍🏽

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

Successfully merging this pull request may close these issues.

Kitchen Sink: Build A Stock Price Tracker
3 participants