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

update dapp code according to tupui's review #37

Merged
merged 15 commits into from
Sep 5, 2024
Merged

Conversation

0xExp-po
Copy link
Contributor

@0xExp-po 0xExp-po commented Sep 4, 2024

  • I've fixed code according to your review

Copy link

netlify bot commented Sep 4, 2024

Deploy Preview for tansu canceled.

Name Link
🔨 Latest commit 38309b6
🔍 Latest deploy log https://app.netlify.com/sites/tansu/deploys/66d962b8c9541e000819b983

Copy link

netlify bot commented Sep 4, 2024

Deploy Preview for testnet-tansu ready!

Name Link
🔨 Latest commit 38309b6
🔍 Latest deploy log https://app.netlify.com/sites/testnet-tansu/deploys/66d962b8d5a94100089f103f
😎 Deploy Preview https://deploy-preview-37--testnet-tansu.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Owner

@tupui tupui left a comment

Choose a reason for hiding this comment

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

Thanks, just a minor thing and it's good to go.

Please push new commits on this branch for updates instead of closing and making a new PR 😃 Otherwise I need to go through the changes again

---

<div class="relative flex flex-col items-center md:flex-row justify-between">
<Topic title="Project Information" description="" id="project-name-topic"/>
Copy link
Owner

Choose a reason for hiding this comment

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

Sorry maybe I was not clear. This should look like this.

Suggested change
<Topic title="Project Information" description="" id="project-name-topic"/>
<Topic title="Project" description="Details about the project" id="project-name-topic"/>

And when you get the name it should appear in another div right above the list of maintainers. So around L25 here.

Can be like you have in the modal to update the config basically.

);

if (updateStatus) {
updateProjectInfo();
Copy link
Owner

Choose a reason for hiding this comment

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

For a follow up, I think it should also reload the project info. And same as with the other buttons, it should add the spinner component so users know something happened after they clicked the button.

dapp/src/components/SetProject.astro Outdated Show resolved Hide resolved
dapp/src/components/utils/Modal.astro Outdated Show resolved Hide resolved
dapp/src/components/ProjectInfo.astro Outdated Show resolved Hide resolved
Copy link
Owner

@tupui tupui left a comment

Choose a reason for hiding this comment

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

Thanks again, looks great 🚀

@tupui tupui merged commit eae0ea6 into tupui:main Sep 5, 2024
8 checks passed
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.

2 participants