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

Basic Account Viewer by firdausfarul #126

Open
2 of 3 tasks
Firdausfarul opened this issue Sep 20, 2021 · 7 comments
Open
2 of 3 tasks

Basic Account Viewer by firdausfarul #126

Firdausfarul opened this issue Sep 20, 2021 · 7 comments

Comments

@Firdausfarul
Copy link
Contributor

Firdausfarul commented Sep 20, 2021

Link the bounty file

https://github.com/tyvdh/stellar-quest-bounties/blob/main/bounties/level-1/basic-account-viewer.md

Mark your progress

  • 🔵  Started working
  • 🟢  Ready for review
  • 🟣  Review completed

Provide relevant details

Repository : https://github.com/Firdausfarul/StellarAccountViewer/tree/master

Demo : https://firdausfarul.github.io/StellarAccountViewer/

@hanseartic
Copy link
Contributor

Thank you for your submission!
I did not go into details, yet but what I caught upfront is the non-functional https connection:
image
Even though everything should be handled in the browser it's essential for the trust to provide a secure connection. I would really appreciate that being changed. Looking forward to take a deeper look later.

@Firdausfarul
Copy link
Contributor Author

Changed the link to default github pages domain so it's https secured. The free domain that came with student dev pack doesn't came with free SSL.

@raeesnazeer
Copy link
Contributor

That Looks good, it passes all the conditions required for the bounty, but the log out button being visible even before logging In makes it look odd. you can try making it visible only once the user logins.

Other than that it's good to go!

@LorDDark6660
Copy link
Contributor

LorDDark6660 commented Dec 11, 2021

if I'm trying to connect from a mobile device with Freighter button, it appears that I'm connected even when I'm not, it would be good to post some error message. I have not Freighter installed.
Screenshot_2021-12-11-22-51-53-10_40deb401b9ffe8e1df2f1cc5ba480b12

@BlackBadPinguin
Copy link
Contributor

BlackBadPinguin commented Dec 15, 2021

For me, it works fine on mobile and desktop @LorDDark6660

Now onto your submission @Firdausfarul, the code looks fine and is well documented, may be some spelling errors in the comments, but apart from that, it looks very decent. It does exactly what has been proposed in the Bounty and the Website is well-formatted. I especially like the logout button which is a nice addition. Also that you have alerts when the specific wallet isn't installed is a nice detail. From my side of things, your submission is ready for a reward.

@alejomendoza
Copy link

alejomendoza commented Dec 16, 2021

@Firdausfarul When you reject the connection with Freighter the ui looks as you are still trying to connect. I like this is super simple viewer, but still some error handling to do. Same as what @LorDDark6660 mentioned.

@BlackBadPinguin
Copy link
Contributor

Are you still working on it?

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

No branches or pull requests

6 participants