feat: created a new crypto page #330
Conversation
|
Have resolved a merge conflict |
|
Hi @karthiknadar1204, |
|
Ok so remove the charts and shift the links of cards in preview1.mp4 to learn more, is that right? |
|
Will do and push changes right away |
|
Also make sure, you don't change anything in the cryptocurrencies page. |
|
Will remove any additional files, I was about to add a loader but then forgot to delete the file. |
|
@royalpinto007 WIll push the changes by 4:15 and ping you |
|
No issues! |
|
@royalpinto007 I have removed the graph, as for creating a section in learn more, I was thinking of keeping the links as it is, for learn more we could add something else in the near future, it could include info about what is crypto and stuff, what do you say, now changing the path to learn more will involve plethora of complex changes in the path. Your opinion? |
I have removed the graph but when pushing the changes, it is showing behind the main branch, tried to pull the changes but still showing error |
|
But the thing is, we have more data already present on the page that has been created. We just need to customize and style it properly. You can refer to this page for more insights on how we actually want to implement things for this page: https://coinmarketcap.com/ We can keep your cards in the "Learn More" section, as they don't provide significant information.
|
|
works |
For this issue, for now merge this pr if possible, in my next or, will remove the graph and add the cards to the learn more link as for the change in links, have to alter the routes in App.jsx and already facing issues with pushing the changes after removing the graph, so at least will pull these changes before pushing more |
|
Dont want to be riddled with unnecessary branch conflicts, the present changes itself involved writing plethora of code, the second cohort of changes should be done latest by tonight or tomorrow afternoon, is that fine @royalpinto007 |
|
Let me check this, thanks for updating! |
|
Could you please create a video showcasing the changes that you have made and add it to the description? |
|
In a min, just got rid of the graph in my change |
|
|
The info could be beautified, but here I was focussed on getting up the main functionality via the api |
|
I agree, but don't you think we can obtain the same data in the form of a table through the TradingView API as well? (present) |
|
Definitely possible, coingecko was easier to integrate and work with with the variety of info it had to offer plus I had experience working with it in earlier projects so thought it was a viable option. |
|
For a tabular view, we can style the incoming info from the API as well, wouldn't make much difference is what I think. |
|
@royalpinto007 Good to go? |
|
@royalpinto007 Any update? |
|
@royalpinto007 Any update? |
|
|
||
| const Cryptocurrencies = () => { | ||
| return ( | ||
| <div> | ||
| <Navbar /> | ||
| <Header /> | ||
| <br /> | ||
| <Widget1 /> |
There was a problem hiding this comment.
Not sure why the space is removed, please update it, thanks!
There was a problem hiding this comment.
An empty file, please delete this, doesn't seem to be a part of this PR!
There was a problem hiding this comment.
Charts aren't supposed to be a part of this as already mentioned, please update this, thanks!
There was a problem hiding this comment.
I'm not quite sure why the change has been made here, adding the same content once again.
Please let me know the reason behind this so that we can proceed further.
Thanks!
There was a problem hiding this comment.
The change isn't supposed to be made to this file. If you want to add a widget, please create a separate file for it instead of modifying the existing file. Please update accordingly. Thanks!
There was a problem hiding this comment.
Again the change isn't supposed to be made to this file. If you want to add a widget, please create a separate file for it instead of modifying the existing file. Please update accordingly. Thanks!
|
Thank you for the review @royalpinto007 |
preview_1.mp4
The above is the preview of the page we access on clicking the "cryptocurrencies" link on navbar. This is perfectly responsive.
preview_2.mp4
The second preview is of the page I have created for individual coins displaying the graph and other info of individual coins,I haven't made this page responsive , Could you please open a separate issue for someone else to make the second page responsive. Hope that's fine.
In the above issue, I have created a basic skeleton of the cryptocurrencies page using the coingecko API, the page still has a lot of scope for improvement , can be done for sure.
The API call might get hindered sometimes as we are using a free api, just go to COinDetails.jsx, comment out the info coming from API, reload and then uncomment..
Closes issue #305