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: Connect Project Overview Page #76

Merged
merged 20 commits into from
May 11, 2022
Merged

Conversation

JoseRFelix
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented May 1, 2022

Visit the preview URL for this PR (updated for commit cd41bdd):

https://ignite-new--pr76-jose-feat-connect-ov-l86ubru8.web.app

(expires Wed, 18 May 2022 17:03:39 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

@fadeev
Copy link
Contributor

fadeev commented May 4, 2022

@JoseRFelix are these issues in the scope of this PR? (it's ok if they're not)

@fadeev fadeev linked an issue May 4, 2022 that may be closed by this pull request
@JoseRFelix
Copy link
Contributor Author

@JoseRFelix are these issues in the scope of this PR? (it's ok if they're not)

Yes!

@fadeev
Copy link
Contributor

fadeev commented May 9, 2022

I know we're getting fundraisers by a coordinator now, but there is an endpoint that will soon be available that will help us get the data we need (fundraiser IDs).

tendermint/spn#770

@JoseRFelix
Copy link
Contributor Author

JoseRFelix commented May 9, 2022

I know we're getting fundraisers by a coordinator now, but there is an endpoint that will soon be available that will help us get the data we need (fundraiser IDs).

tendermint/spn#770

Just saw this; what I'll do for now is hide the fundraiser card and in another PR add this functionality after updating the query client. I don't want to make this PR have >5K lines

@JoseRFelix JoseRFelix marked this pull request as ready for review May 9, 2022 19:30
@JoseRFelix JoseRFelix linked an issue May 10, 2022 that may be closed by this pull request
@fadeev
Copy link
Contributor

fadeev commented May 11, 2022

@Matthews3301 @marinhoarthur please, review.

@marinhoarthur
Copy link
Contributor

Can we replace addCommasToNumber(number) w/ <IgniteNumber :number="number">? This way we obey user's locale.

@marinhoarthur
Copy link
Contributor

marinhoarthur commented May 11, 2022

Overall is tight.

I think we should gracefully fail if either .md or .yml is 404. Right now we're just showing blank. Asked on Figma about an empty state.

@marinhoarthur
Copy link
Contributor

marinhoarthur commented May 11, 2022

Can we replace addCommasToNumber(number) w/ <IgniteNumber :number="number">? This way we obey user's locale.

Only this and we're good to go imo @JoseRFelix .

@JoseRFelix
Copy link
Contributor Author

Can we replace addCommasToNumber(number) w/ <IgniteNumber :number="number">? This way we obey user's locale.

Thanks! Removed all instances of addCommasToNumber in favor of this 👍

@JoseRFelix
Copy link
Contributor Author

Overall is tight.

I think we should gracefully fail if either .md or .yml is 404. Right now we're just showing blank. Asked on Figma about an empty state.

Yes, there is an issue with it #93. I'll work on it

@JoseRFelix
Copy link
Contributor Author

@marinhoarthur Ready to be re-reviewed

src/utils/fundraising.ts Show resolved Hide resolved
@JoseRFelix JoseRFelix merged commit 6c6d6c1 into main May 11, 2022
@JoseRFelix JoseRFelix deleted the jose/feat/connect-overview-page branch May 11, 2022 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants