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

Refactor some stuff #428

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Conversation

panleone
Copy link
Member

Abstract

A bit of refactor, I suggest to review commit by commit

  • first commit: remove the wallet from the network constructor;
  • second commit: add a use_network.js class, intermediate between VUE and .js logic
  • third commit: use the new composable class in place of importing the global network class
  • fourth commit: clean up unused imports and a function in global.js
  • fifth commit: get rid of the global block count in Activity.js

@panleone panleone added the Refactor A PR or suggestion for rewriting existing code. label Oct 14, 2024
@panleone panleone self-assigned this Oct 14, 2024
Copy link

netlify bot commented Oct 14, 2024

Deploy Preview for cheery-moxie-4f1121 ready!

Name Link
🔨 Latest commit 1edef67
🔍 Latest deploy log https://app.netlify.com/sites/cheery-moxie-4f1121/deploys/670f59b8bc02000008f14c85
😎 Deploy Preview https://deploy-preview-428--cheery-moxie-4f1121.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
Member

@Duddino Duddino left a comment

Choose a reason for hiding this comment

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

other than that nit should be good to merge

import { defineStore } from 'pinia';

export const useNetwork = defineStore('network', () => {
const explorerUrl = ref('');
Copy link
Member

Choose a reason for hiding this comment

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

Either watch the variable and change explorer or set is as read only.

Copy link
Member

@JSKitty JSKitty left a comment

Choose a reason for hiding this comment

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

openExplorer was used by the Governance Tab for opening addresses, it'll need replacing with something (until use_network.js can be used with Duddino's Vue Governance port)

image

@panleone
Copy link
Member Author

openExplorer was used by the Governance Tab for opening addresses, it'll need replacing with something (until use_network.js can be used with Duddino's Vue Governance port)

image

nice catch, I don't know why I thought it was unused

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Refactor A PR or suggestion for rewriting existing code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants