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

Jordan/1502 page proposal #1531

Merged
merged 23 commits into from
Nov 9, 2018
Merged

Jordan/1502 page proposal #1531

merged 23 commits into from
Nov 9, 2018

Conversation

jbibla
Copy link
Collaborator

@jbibla jbibla commented Nov 7, 2018

Closes #1502

Description:

implemented page proposal component and included some nice upgrades to proposals index and a couple of other places.

❤️ Thank you!


  • Added entries in CHANGELOG.md with issue # and GitHub username
  • Reviewed Files changed in the github PR explorer

@jbibla jbibla changed the title Jordan/1502 page proposal WIP: Jordan/1502 page proposal Nov 7, 2018
@codecov
Copy link

codecov bot commented Nov 7, 2018

Codecov Report

Merging #1531 into develop will increase coverage by 0.04%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop    #1531      +/-   ##
===========================================
+ Coverage    96.41%   96.45%   +0.04%     
===========================================
  Files           97       98       +1     
  Lines         1811     1833      +22     
  Branches        86       91       +5     
===========================================
+ Hits          1746     1768      +22     
  Misses          55       55              
  Partials        10       10
Impacted Files Coverage Δ
.../src/renderer/components/staking/PageValidator.vue 98.82% <ø> (ø) ⬆️
app/src/renderer/components/common/TmBalance.vue 100% <ø> (ø) ⬆️
.../renderer/components/governance/TableProposals.vue 100% <ø> (ø) ⬆️
app/src/renderer/routes.js 100% <ø> (ø) ⬆️
app/src/renderer/connectors/lcdClientMock.js 99.39% <ø> (ø) ⬆️
...rc/renderer/components/governance/PageProposal.vue 100% <100%> (ø)
.../src/renderer/components/governance/LiProposal.vue 100% <100%> (ø) ⬆️

return `Voting for this proposal is open`

if (this.proposal.proposal_status === `Pending`)
return `Deposits are open for this proposal`
Copy link
Contributor

@fedekunze fedekunze Nov 8, 2018

Choose a reason for hiding this comment

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

Deposits are always open as long as the proposal is open (i.e you can also deposit when the proposal is Active), in this case the proposal is Pending because it hasn't met the minimum deposit

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

why would a user deposit if a proposal has received enough to open voting?

Copy link
Contributor

Choose a reason for hiding this comment

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

I also said that... But you can ask @sunnya97

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we'll see what sunny says, but i don't think we should block on this.

@jbibla jbibla changed the title WIP: Jordan/1502 page proposal Jordan/1502 page proposal Nov 8, 2018
@@ -94,6 +94,12 @@ export default {
tooltip: `The title of the proposal`,
class: `proposal_title`
},
{
title: `Submitted Block`,
Copy link
Contributor

Choose a reason for hiding this comment

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

sorting by proposal id is the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

true, but i added a column for submit block instead. do you think showing ID as its own column would be better?

Copy link
Contributor

Choose a reason for hiding this comment

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

we can discuss it later

<p>
Status: Passed
Proposal description…
Copy link
Contributor

Choose a reason for hiding this comment

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

Add the ... only if the length is > 100

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

smart! thanks @fedekunze

// },

{
path: `/governance/:proposalId`,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd change this to

Suggested change
path: `/governance/:proposalId`,
path: `/governance/proposals/:proposalId`,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

how come?

Available Steak
</h3>
<h2>
0.0000…
Copy link
Contributor

Choose a reason for hiding this comment

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

is this a description ? why does it have the ... ?

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 is how we show numbers in the balance header!

Yes
</dt>
<dd>
500 / 0%
Copy link
Contributor

Choose a reason for hiding this comment

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

0 % ?

No
</dt>
<dd>
25 / 0%
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

No with Veto
</dt>
<dd>
10 / 0%
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Abstain
</dt>
<dd>
56 / 0%
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

Total Steak
</h3>
<h2>
0.0000…
Copy link
Contributor

Choose a reason for hiding this comment

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

...

statusColor() {
if (this.proposal.proposal_status === `Rejected`) return `red`
else if (this.proposal.proposal_status === `Passed`) return `green`
status() {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

does this cover all the cases / is this true?

@fedekunze fedekunze mentioned this pull request Nov 9, 2018
7 tasks
Copy link
Contributor

@fedekunze fedekunze left a comment

Choose a reason for hiding this comment

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

tested ACK

@fedekunze fedekunze merged commit 2786be1 into develop Nov 9, 2018
@fedekunze fedekunze deleted the jordan/1502-page-proposal branch November 9, 2018 15:23
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