liquidvotingio / decidim-module-liquidvoting

GNU Affero General Public License v3.0
4 stars 0 forks source link

Replace support count and 'support' button with 'view proposal' button, in proposals index view proposal cards #106

Closed jinjagit closed 3 years ago

jinjagit commented 3 years ago

Resolves #104

Temporarily remove support count and support button from proposal cards footers in proposals index view. The 'card-footer' is not used in the proposal show view.

Note: 'Amendment' cards, which may also be shown in the proposals index view, and the process show view, never show the 'View Proposal' button, and this is also the case in the original Decidim (non-liquidvoting) version.

This avoids updating the support count and support button state for each proposal shown in the index view, and also avoids adding our delegation UI to index view proposal cards.

We may well want to add this functionality back in when we have completed other demo features, but for now this allows us to move forward without presenting wrong / mismatched information to a user.

Here's the before view: it's confusing to present Support without Delegation

Screenshot 2021-04-27 at 15 34 28

And here's the After, where we let the user click to the detail page (which has vote/delegate)

Screenshot 2021-04-27 at 15 37 13
jinjagit commented 3 years ago

Screenshot of proposals index view (note: 'amendment' cards, which may also be shown in the proposals index view, and the process show view, never show the 'View Proposal' button, and this is also the case in the original Decidim, non-liquidvoting, version):

proposals_index

jinjagit commented 3 years ago

@davefrey

Some general PR comments: coming to the PR I looked first for the issue number (what I read first in a PR)...something like "Resolves #104" in the PR description is ideal if it's known when the PR is created.

Done.

Second, if I can have a visual clue to the changes, like a before and after screenshot of a card, that dials me into the PR quickly.

Good idea. But... do you have an image of the proposal card before the change? I didn't think to capture one. EDIT: I'll bypass the change and capture the 'before' state.

davefrey commented 3 years ago

I added the before/after images to the description (you can also always get back to "before" by checking out master, or eventually by running the app from a decidim v0.24.0 workspace).

But now that I see the images ... why did we remove the support counts? It seems to add value without adding confusion, and since we override the footer we can do what we want.

jinjagit commented 3 years ago

@davefrey

But now that I see the images ... why did we remove the support counts? It seems to add value without adding confusion, and since we override the footer we can do what we want.

I chose to remove them, as they are not shown in the process overview page, which presents a selection of proposal cards for the process, with the same 'View Proposal' button on the card footers. Thus, I felt it would be less jarring to the user.

Also, I would need to confirm the count is correct in all cases in this index view (including when we have used our API in various ways to alter it), unless you are already sure of this. I also side-stepped this work by not including the count.

davefrey commented 3 years ago

~Ah good point about retrieving all the relevant counts, you're exactly right~ Edit: we should normally trust the Proposal#proposal_vote_count, right? The cell has the proposal, and its count is the most up-to-date

jinjagit commented 3 years ago

Yes, the fact that they are not included when cards are in other view + have 'View proposal' button gave me a good way to avoid this issue, for now.

We can, of course, re-include the count if/when we have the ajax for 'support' button + delegation UI working on the cards (if we wish to do this).

davefrey commented 3 years ago

👍 Consistency with other View Proposal cards, I agree

jinjagit commented 3 years ago

@dave @oliverbarnes I just corrected a one-word typo in a comment, after re-requesting review.