kredits/kredits-web
9
0
Fork 0
Code Issues 22 Pull Requests 1 Actions Releases 18 Activity

Veto contributions #113

Merged
raucao merged 5 commits from feature/98-veto_unconfirmed_contributions into master 2019-05-01 18:43:29 +00:00
Conversation 9 Commits 5 Files Changed 13
+174 -63
raucao commented 2019-04-28 14:27:29 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Screenshot from 2019-04-28 15-04-39

closes #98

![Screenshot from 2019-04-28 15-04-39](https://user-images.githubusercontent.com/842/56865813-2b2a1800-69ca-11e9-803e-d2ddd40e8ba1.png) closes #98
raucao (Migrated from github.com) reviewed 2019-04-28 14:31:13 +00:00
app/components/contribution-list/template.hbs
@@ -1,5 +1,5 @@
{{#each contributions as |contribution|}}
<li data-contribution-id={{contribution.id}} class={{if contribution.confirmed "confirmed" "unconfirmed"}}>
<li data-contribution-id={{contribution.id}} class="{{contribution-status contribution}} {{if contribution.vetoed "vetoed"}}">
<p class="meta">
raucao (Migrated from github.com) commented 2019-04-28 14:31:12 +00:00
Copy Link
Copy Source

This is a bit of a hack. It shouldn't required the if for the second class. But apparently with the helper, it doesn't bind the contribution property updates, so the value stays the same after setting the contribution vetoed to true. :/

@galfert @fsmanuel Any idea if that's an inherent limitation of helpers, or if I did something wrong?

This is a bit of a hack. It shouldn't required the `if` for the second class. But apparently with the helper, it doesn't bind the contribution property updates, so the value stays the same after setting the contribution `vetoed` to true. :/ @galfert @fsmanuel Any idea if that's an inherent limitation of helpers, or if I did something wrong?
galfert (Migrated from github.com) reviewed 2019-04-29 11:48:39 +00:00
app/components/contribution-list/template.hbs
@@ -1,5 +1,5 @@
{{#each contributions as |contribution|}}
<li data-contribution-id={{contribution.id}} class={{if contribution.confirmed "confirmed" "unconfirmed"}}>
<li data-contribution-id={{contribution.id}} class="{{contribution-status contribution}} {{if contribution.vetoed "vetoed"}}">
<p class="meta">
galfert (Migrated from github.com) commented 2019-04-29 11:48:39 +00:00
Copy Link
Copy Source

Mmh, indeed it looks like when passing objects to a helper, it doesn't recalculate when one of the object's properties changes.

I see two options:

  1. Pass the two properties that the helper is using directly as parameters (contribution-status contribution.vetoed contribution.confirmedAt).

  2. Create a component for the contribution (e.g. contribution-item). This way the component can have a computed property for the class.

Mmh, indeed it looks like when passing objects to a helper, it doesn't recalculate when one of the object's properties changes. I see two options: 1. Pass the two properties that the helper is using directly as parameters (`contribution-status contribution.vetoed contribution.confirmedAt`). 2. Create a component for the contribution (e.g. `contribution-item`). This way the component can have a computed property for the class.
raucao (Migrated from github.com) reviewed 2019-04-29 12:16:35 +00:00
app/components/contribution-list/template.hbs
@@ -1,5 +1,5 @@
{{#each contributions as |contribution|}}
<li data-contribution-id={{contribution.id}} class={{if contribution.confirmed "confirmed" "unconfirmed"}}>
<li data-contribution-id={{contribution.id}} class="{{contribution-status contribution}} {{if contribution.vetoed "vetoed"}}">
<p class="meta">
raucao (Migrated from github.com) commented 2019-04-29 12:16:35 +00:00
Copy Link
Copy Source

Thanks!

I think version 2 is better, but that it should be done when refactoring the UI soon. I think we'll want to use something like ember-boostrap instead of our handrolled everything, because otherwise we spend too much time re-inventing the wheel for all the various UI components.

Thanks! I think version 2 is better, but that it should be done when refactoring the UI soon. I think we'll want to use something like `ember-boostrap` instead of our handrolled everything, because otherwise we spend too much time re-inventing the wheel for all the various UI components.
galfert (Migrated from github.com) reviewed 2019-04-29 12:17:46 +00:00
app/components/contribution-list/template.hbs
@@ -1,5 +1,5 @@
{{#each contributions as |contribution|}}
<li data-contribution-id={{contribution.id}} class={{if contribution.confirmed "confirmed" "unconfirmed"}}>
<li data-contribution-id={{contribution.id}} class="{{contribution-status contribution}} {{if contribution.vetoed "vetoed"}}">
<p class="meta">
galfert (Migrated from github.com) commented 2019-04-29 12:17:46 +00:00
Copy Link
Copy Source

I like version 2 better as well.

I like version 2 better as well.
bumi (Migrated from github.com) reviewed 2019-05-01 18:31:56 +00:00
bumi (Migrated from github.com) left a comment
Copy Link
Copy Source

I can not say much about the ember code, but looks good to me.

the UI looks and works great!

I can not say much about the ember code, but looks good to me. the UI looks and works great!
app/helpers/contribution-status.js
@@ -0,0 +7,4 @@
kredits: service(),
currentBlock: alias('kredits.currentBlock'),
compute([contribution]) {
bumi (Migrated from github.com) commented 2019-05-01 18:25:07 +00:00
Copy Link
Copy Source

can this also be something on the model? (#rubythinking :)

can this also be something on the model? (#rubythinking :)
app/services/kredits.js
@@ -56,3 +56,4 @@
console.debug('[kredits] Using user-provided instance, e.g. from Mist browser or Metamask');
ethProvider = new ethers.providers.Web3Provider(web3Provider);
// const network = await ethProvider.getNetwork();
ethProvider.listAccounts().then(accounts => {
bumi (Migrated from github.com) commented 2019-05-01 18:25:24 +00:00
Copy Link
Copy Source

we can delete this comment.

we can delete this comment.
bumi (Migrated from github.com) approved these changes 2019-05-01 18:32:23 +00:00
raucao (Migrated from github.com) reviewed 2019-05-01 18:36:21 +00:00
app/helpers/contribution-status.js
@@ -0,0 +7,4 @@
kredits: service(),
currentBlock: alias('kredits.currentBlock'),
compute([contribution]) {
raucao (Migrated from github.com) commented 2019-05-01 18:36:21 +00:00
Copy Link
Copy Source

No, because you cannot inject services into a model.

No, because you cannot inject services into a model.
raucao (Migrated from github.com) reviewed 2019-05-01 18:38:37 +00:00
app/services/kredits.js
@@ -56,3 +56,4 @@
console.debug('[kredits] Using user-provided instance, e.g. from Mist browser or Metamask');
ethProvider = new ethers.providers.Web3Provider(web3Provider);
// const network = await ethProvider.getNetwork();
ethProvider.listAccounts().then(accounts => {
raucao (Migrated from github.com) commented 2019-05-01 18:38:36 +00:00
Copy Link
Copy Source

I left it there intentionally, so that I don't have to look up how to get to the network info of the external provider, and where to do that, when tackling #82. (Was going to do a drive-by PR for that, but defining which network is the right one isn't straight-forward.)

I left it there intentionally, so that I don't have to look up how to get to the network info of the external provider, and where to do that, when tackling #82. (Was going to do a drive-by PR for that, but defining which network is the right one isn't straight-forward.)
bumi commented 2019-05-01 18:40:49 +00:00 (Migrated from github.com)
Copy Link
Copy Source

good for me to merge... @galfert?

good for me to merge... @galfert?
🎉 1
Sign in to join this conversation.
Reviewers
No Reviewers
galfert
bumi
raucao
Labels
Clear labels
bug
Something is not working
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No Label
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) haythem (haythem) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kredits/kredits-web#113
Delete Branch "feature/98-veto_unconfirmed_contributions"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?