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

Save deployed contracts #4524

Merged
merged 25 commits into from
Feb 26, 2024
Merged

Save deployed contracts #4524

merged 25 commits into from
Feb 26, 2024

Conversation

Aniket-Engg
Copy link
Collaborator

@Aniket-Engg Aniket-Engg commented Feb 9, 2024

This allow saving a contract for injected web3 provider

Related to #3343

Copy link

netlify bot commented Feb 9, 2024

Deploy Preview for remixproject ready!

Name Link
🔨 Latest commit 2835f6f
🔍 Latest deploy log https://app.netlify.com/sites/remixproject/deploys/65dc89bd0ec50800081eb807
😎 Deploy Preview https://deploy-preview-4524--remixproject.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.

@Aniket-Engg Aniket-Engg changed the title add save icon Save deployed contracts Feb 9, 2024
@Aniket-Engg Aniket-Engg added the WIP label Feb 9, 2024
@Aniket-Engg Aniket-Engg force-pushed the saveDeplCont branch 2 times, most recently from 3ac6269 to 546a6aa Compare February 21, 2024 14:22
@Aniket-Engg Aniket-Engg added ready-to-review PR ready to review and removed WIP labels Feb 22, 2024
useEffect(() => {
const fetchSavedContracts = async () => {
env.current = await props.plugin.call('blockchain', 'getProvider')
if(env.current && env.current === 'injected') {
Copy link
Contributor

Choose a reason for hiding this comment

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

could this be for all providers which aren't a Remix VM?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This should be discussed

@Aniket-Engg Aniket-Engg force-pushed the saveDeplCont branch 3 times, most recently from b70fac7 to ba24793 Compare February 23, 2024 07:46
@Aniket-Engg Aniket-Engg requested a review from yann300 February 23, 2024 07:46
@Aniket-Engg Aniket-Engg force-pushed the saveDeplCont branch 2 times, most recently from def44d1 to c30a18b Compare February 26, 2024 06:18
@yann300
Copy link
Contributor

yann300 commented Feb 26, 2024

Saved contracts don't work while using the AtAddress feature

@yann300
Copy link
Contributor

yann300 commented Feb 26, 2024

when switching networks, only the saved contracts from the current network should be displayed.

@LianaHus
Copy link
Collaborator

  1. As we discussed previously in the initial call, "Currently, you have no saved contracts to interact with." It takes too much space. the one for deployed contracts was fine as long as there was nothing shown after that. now we have 2 huge fields there that attract too much unnecessary attention. Maybe try to remove the alert and leave a text warning or info at least saved once.

  2. There are issues in the flow.

  • First of all, once the user saves, we remove the TX from the second list. it is confusing and we destroy the order.
  • Second - once we remove it from the "saved" list, it doesn't come back to the "Deployed.." list but disappears.

@Aniket-Engg
Copy link
Collaborator Author

when switching networks, only the saved contracts from the current network should be displayed.

this is done

@Aniket-Engg
Copy link
Collaborator Author

  1. As we discussed previously in the initial call, "Currently, you have no saved contracts to interact with." It takes too much space. the one for deployed contracts was fine as long as there was nothing shown after that. now we have 2 huge fields there that attract too much unnecessary attention. Maybe try to remove the alert and leave a text warning or info at least saved once.
  2. There are issues in the flow.
  • First of all, once the user saves, we remove the TX from the second list. it is confusing and we destroy the order.
  • Second - once we remove it from the "saved" list, it doesn't come back to the "Deployed.." list but disappears.

point 2 is done

@yann300
Copy link
Contributor

yann300 commented Feb 26, 2024

the time displayed for the saved on property shows GMT timezone, it would be better to show the current timezone. (probably not for this PR though)

@Aniket-Engg
Copy link
Collaborator Author

Saved contracts don't work while using the AtAddress feature

I think this feature is also working

@Aniket-Engg Aniket-Engg merged commit 60e200b into master Feb 26, 2024
32 checks passed
@Aniket-Engg Aniket-Engg deleted the saveDeplCont branch February 26, 2024 17:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants