-
Notifications
You must be signed in to change notification settings - Fork 116
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
[Governance] proposal metadata from url #47
Conversation
@@ -21,6 +21,25 @@ export function ProposalContent({ | |||
return null; | |||
} | |||
|
|||
const [metadata, setMetadata] = useState<any>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add a type instead of any
(e.g. it appears to be an object that has description
and execution_script
properties)?
}; | ||
|
||
fetchMetadata(); | ||
}, []); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it re-fetch the metadata if metadata_location
changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the comment! metadata_location
comes from the chain so it's probably won't be changed. Although I'll apply the change to be safe.
const CODE_LOCATION_PLACEHOLDER = "https://aptoslabs.com/"; | ||
const TITLE_PLACEHOLDER = "Title -- Lorem ipsum dolor sit amet"; | ||
const METADATA_LOCATION_PLACEHOLDER = | ||
"https://mocki.io/v1/32403c87-d5d2-4136-80fd-d639a5d3d7dd"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
are we also getting the title from the metadata link? https://mocki.io/v1/32403c87-d5d2-4136-80fd-d639a5d3d7dd has title also
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned in the PR summary, I'll add the title later to avoid merge conflicts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
To align with change in aptos-labs/aptos-core#2121, this PR display proposal content metadata (description, code) from and external url.
Next step: