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.

Sign up for GitHub

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

Jump to bottom

Review added for the 318-4:5 #227

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Review added for the 318-4:5 #227

wants to merge 3 commits into from

Conversation

furkanahmetk
Copy link
Contributor

review added and updated for the grant #318 milestones 4&5.

ggurbet
ggurbet reviewed Nov 20, 2023
View reviewed changes
...nal feedback - Token Bridge & Incorporate audit feedback and launch - Token Bridge/README.md

[test logs](assets/maketest(1).txt)

![screenshot of successfull testing](test.jpeg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
![screenshot of successfull testing](test.jpeg)
![screenshot of successfull testing](assets/test.jpeg)

ggurbet
ggurbet reviewed Nov 20, 2023
View reviewed changes
...nal feedback - Token Bridge & Incorporate audit feedback and launch - Token Bridge/README.md

There is no main page button or logo of ferrum does not leads user to main page so user needs to refresh the page to go to main page when s/he is on the withdrawl or add liquidity pages. When user refresh the page it gives an erros then leads you to main page but everything will be resetted and user will needs to connect wallets again. Which effects user experience negatively ,so the reviewer strongly recommends to fix this.

![Refreshing page issue](assets/refrespage.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
![Refreshing page issue](assets/refrespage.png)
![Refreshing page issue](assets/refreshpage.png)

ggurbet
ggurbet suggested changes Nov 20, 2023
View reviewed changes
Copy link
Contributor

@ggurbet ggurbet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Suggested some fixes for the typos in some of the file paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ggurbet ggurbet ggurbet requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@furkanahmetk @ggurbet