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

Converter - update UI #86

Merged
merged 22 commits into from
Jul 6, 2023
Merged

Converter - update UI #86

merged 22 commits into from
Jul 6, 2023

Conversation

abbylow
Copy link
Contributor

@abbylow abbylow commented Jul 5, 2023

SCR-150 remove "Welcome to the Converter" title
SCR-149 display conversion details without wallet connection
SCR-148 remove accept terms screen
fix hydration error for Transaction Panel and Smart Contract Tracker
SCR-136 display toast for getting back to pending conversion screen

@abbylow abbylow requested a review from jasheal as a code owner July 5, 2023 06:47
@vercel
Copy link

vercel bot commented Jul 5, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
mantle-converter ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 6, 2023 0:55am
5 Ignored Deployments
Name Status Preview Comments Updated (UTC)
ethcc ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2023 0:55am
mantle-bridge ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2023 0:55am
mantle-faucet ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2023 0:55am
mantle-nft ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2023 0:55am
mantle-token-graph ⬜️ Ignored (Inspect) Visit Preview Jul 6, 2023 0:55am

pnpm-lock.yaml Outdated Show resolved Hide resolved
apps/converter/src/components/converter/Convert.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@jasheal jasheal left a comment

Choose a reason for hiding this comment

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

Great work - I’ll review the details. For next time if we can make PRs a lot smaller? It’s ok to group multiple items if they are all simple text changes but mixing simple text and conditional logic / functional changes on 1 PR is hard to review and can lead to conflicts.

@Rieranthony
Copy link
Contributor

Great work - I’ll review the details. For next time if we can make PRs a lot smaller? It’s ok to group multiple items if they are all simple text changes but mixing simple text and conditional logic / functional changes on 1 PR is hard to review and can lead to conflicts.

The PR length will be solved by removing the new pnpm.lock file, I gave her the instructions to fix it ✌️

@abbylow
Copy link
Contributor Author

abbylow commented Jul 5, 2023

Great work - I’ll review the details. For next time if we can make PRs a lot smaller? It’s ok to group multiple items if they are all simple text changes but mixing simple text and conditional logic / functional changes on 1 PR is hard to review and can lead to conflicts.

Noted. Initially I grouped these few tasks in one PR as they are simple UI updates. Unexpectedly updated more files as the need to solve hydration error and the pnpm-lock file update. I have removed the changes of pnpm-lock file so the PR length should be shorter now.

I will submit smaller PR in the future. Thanks for reviewing it 😉

@Rieranthony Rieranthony self-requested a review July 5, 2023 12:38
Copy link
Contributor

@Rieranthony Rieranthony left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the changes 🎉

@abbylow
Copy link
Contributor Author

abbylow commented Jul 5, 2023

@Rieranthony @jasheal @grezle
I have pushed a new commit to address hydration issue for MNT balance. Sorry for making this PR so long but this issue should be fixed before merging.

@abbylow abbylow changed the title Converter - update UI WIP: Converter - update UI Jul 5, 2023
@abbylow abbylow changed the title WIP: Converter - update UI Converter - update UI Jul 5, 2023
@abbylow abbylow merged commit f920646 into main Jul 6, 2023
2 checks passed
@abbylow abbylow deleted the converter-update-ui branch July 6, 2023 14:26
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.

3 participants