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

Adding refresh functionality to input page #9

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

LukasBudach
Copy link
Contributor

While working on this project I got the idea, that maybe having to refresh all of the ticker's quotes by clicking into the input field and out of it again was not the way people would want to use it.

This is why I added one button to refresh all of the table's prices (circled blue) and one per row to refresh only that row (circled red).

Even though it looks like it in the changes, I did not adapt anything in the bootstrap.min.css file, but removing the last line referencing the source map file. Same for the bootstrap.bundle.min.js.

grafik

@jjacobson
Copy link
Owner

I'll consider this although I don't see why refreshing prices would be necessary considering people would ideally only spend a few seconds on the page entering the initial data. The only case I could see updating prices being necessary is when saved data is loaded when the page is returned to.

@LukasBudach
Copy link
Contributor Author

LukasBudach commented Jan 23, 2019

I understand.
The idea with this feature was to allow users to not only use this calculator to figure out the composition of their portfolio, but also allowing them to use it as somewhat of a portfolio manager where they can keep track of their holdings.
If you're not looking to provide this functionality, as it clearly reaches beyond the scope of your initial project, I'd be happy to use them solely on my fork and cherry-pick those changes I make, that are beneficial to your project and only do pull request on those.

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.

2 participants