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

Add Dynamic Cryptocurrency Selection and Display #46

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

Conversation

ivelinmegdanov
Copy link
Contributor

@ivelinmegdanov ivelinmegdanov commented Jul 15, 2024

Introduces a feature to dynamically select and display cryptocurrency data (Bitcoin and Ethereum) on the page.

  • Added a toggle switch for selecting Bitcoin and Ethereum.
  • Added event listeners for the cryptocurrency toggle switch.
  • Updates the label text based on the selected cryptocurrency.
  • Fetches and displays current price data immediately upon changing cryptocurrency type.
  • Modified data fetching logic to dynamically handle the selected cryptocurrency type.
  • Ensured the /now endpoint is called whenever the cryptocurrency type is change
  • Renamed bitcoin.js to crypto.js

!!! TODO: Inside background.js we need to update to the deployed API url

};

try {
const response = await fetch(`http://localhost:3000/v1/${endpoints[period]}`); // TODO: Update to use the deployed API (maybe use a config file to store the base URL)
Copy link
Owner

Choose a reason for hiding this comment

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

🙄

@superKalo superKalo linked an issue Dec 8, 2024 that may be closed by this pull request
<div class="toggle-switch text-center mb+">
<label> <input type="radio" name="crypto" value="bitcoin" checked /> Bitcoin </label>
<label> <input type="radio" name="crypto" value="ethereum" /> Ethereum </label>
</div>
Copy link
Owner

Choose a reason for hiding this comment

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

I see your point, and I tend to like it at one side, but on another - I prefer much more to see this in the future #54 , instead as a playable call-to-action here.

@@ -9,7 +9,7 @@

<meta
name="description"
content="Replace your browser New Tab page with a Bitcoin price chart"
content="Replace your browser New Tab page with a Crypto price chart"
Copy link
Owner

Choose a reason for hiding this comment

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

Would be great to display Bitcoin or Ethereum in here.

<h2 class="chart-period mb text-center">
Bitcoin price is
<span id="crypto-type">Crypto</span> price is
Copy link
Owner

Choose a reason for hiding this comment

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

@@ -123,6 +128,7 @@ <h2 class="chart-period mb text-center">
<a href="https://github.com/superKalo/crypto-tab">Open Source</a>
|
<a href="https://github.com/cmihaylov/bitcoin-price-api">API</a>
<!-- TODO: Change the source to the new API -->
Copy link
Owner

Choose a reason for hiding this comment

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

The new API is in another repo?

@@ -104,58 +139,61 @@ window.App.Bitcoin = {
const storageSetting =
App.ENV.platform === 'EXTENSION' ? 'BROWSER_STORAGE' : 'LOCAL_STORAGE';

Object.keys(this.PERIODS).forEach((period) => {
this.repositories[period] = new SuperRepo({
['bitcoin', 'ethereum'].forEach((cryptoType) => {
Copy link
Owner

Choose a reason for hiding this comment

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

That's kind of a two-sided performance problem. First, SuperRepo instances are created for both tokens (although only one could be active at a time), and second (more importantly) - the inactive one will still tick-tack and fire API calls for both - the currently active and the inactive tokens.

@superKalo
Copy link
Owner

On HODL until the API problems are resolved and it is deployed LIVE.

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.

Wen Ethereum ticker?
2 participants