-
Notifications
You must be signed in to change notification settings - Fork 4
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
feat: improve proposal details visualization #260
Conversation
@@ -79,6 +65,15 @@ const ProposalDetails: React.FC<ProposalDetailsProps> = ({ proposal }) => { | |||
return ( | |||
<View> | |||
<Spacer paddingTop={24} /> | |||
{/* Summary */} |
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.
@RiccardoM I display the proposal summary above the description. Let me know if this is fine.
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.
Yea it's perfect 👍
Codecov ReportAll modified and coverable lines are covered by tests ✅ 📢 Thoughts on this report? Let us know! |
SonarQube Quality Gate Result
updated: 11/7/2023, 16:47:34 (UTC+0) |
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.
Looks good to me. Just a minor comment on the rendering of the parameter values
serializedValue = JSON.stringify(objectValue, undefined, 4); | ||
} | ||
} else { | ||
serializedValue = objectValue.toString(); |
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.
Does this work on boolean values as well? I saw in your preview Gif that the send_enabled
parameter was rendered as a white square box for some reason.
Also, would it be better to check if the objectValue
is either undefined
or null
and render it as null
in both cases?
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.
@RiccardoM I have handled the case where objectValue
is undefined
or null
.
About the boolean there was an error in the conversion of the /desmos.tokenfactory.v1.MsgUpdateParams
.
I have update it to matche the proto definition.
Description
Closes: DPM-162, DPM-163, DPM-172, DPM-178
This PR enhances the visualization of the details of a governance proposal with the following changes:
MsgUpdateParams
to display the changed parameters.###Final result:
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
Reviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.