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

FrontEnd vote action UI/implementation #15

Merged
merged 23 commits into from
Oct 27, 2023

Conversation

antopalidi
Copy link
Member

@codecov
Copy link

codecov bot commented Oct 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (graded_proposal_voting@e822308). Click here to learn what that means.

Additional details and impacted files
@@                    Coverage Diff                    @@
##             graded_proposal_voting      #15   +/-   ##
=========================================================
  Coverage                          ?   96.41%           
=========================================================
  Files                             ?      120           
  Lines                             ?     3346           
  Branches                          ?        0           
=========================================================
  Hits                              ?     3226           
  Misses                            ?      120           
  Partials                          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

@microstudi microstudi left a comment

Choose a reason for hiding this comment

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

Thanks Anna,
I've made you a pre-review so you can fix some thinks before you go further away!

See the comments but main thinks:

  • we need to isolate the specific implementation of the voting (in this case ThreeFlags)
  • If we can, better not to use channels and websockets (this adds requirements to the server). I really think it is not necessary as the dynamic rendering of the votes results is already implemented in javascript by decidim.
  • If we use CSS, we need to namespace it too

@@ -0,0 +1,93 @@
$rectangle-width: 7rem;

Choose a reason for hiding this comment

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

Suggested change
$rectangle-width: 7rem;
.voting-three-flag {
$rectangle-width: 7rem;

we need to namespace all classes here, we need to take into account that this is only one implementation of the possibles.

@@ -2,6 +2,7 @@
@import "stylesheets/decidim/decidim_awesome/editors/markdown_editor";
@import "stylesheets/decidim/decidim_awesome/editors/quill_editor";
@import "stylesheets/decidim/decidim_awesome/forms/autosave";
@import "stylesheets/decidim/decidim_awesome/vote/vote";

Choose a reason for hiding this comment

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

Suggested change
@import "stylesheets/decidim/decidim_awesome/vote/vote";
@import "stylesheets/decidim/decidim_awesome/voting/three_flags";

Better be more specific, in case we add more systems

Comment on lines 3 to 5
module Decidim
module Proposals
class VoteProposalCell < Decidim::ViewModel
Copy link

@microstudi microstudi Oct 16, 2023

Choose a reason for hiding this comment

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

Suggested change
module Decidim
module Proposals
class VoteProposalCell < Decidim::ViewModel
module Decidim
module DecidimAwesome
module Voting
class ThreeFlagsProposalCell < Decidim::ViewModel

We need to isolate this voting method system. In case we add others (that we will)

Comment on lines 3 to 5
module Decidim
module Proposals
class VoteProposalModalCell < Decidim::ViewModel
Copy link

@microstudi microstudi Oct 16, 2023

Choose a reason for hiding this comment

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

Suggested change
module Decidim
module Proposals
class VoteProposalModalCell < Decidim::ViewModel
module Decidim
module DecidimAwesome
module Voting
class ThreeFlagsProposalModalCell < Decidim::ViewModel

Here the same

Comment on lines 3 to 6
module Decidim
module Proposals
class VotesCounterCell < Decidim::ViewModel
include Decidim::IconHelper
Copy link

@microstudi microstudi Oct 16, 2023

Choose a reason for hiding this comment

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

Suggested change
module Decidim
module Proposals
class VotesCounterCell < Decidim::ViewModel
include Decidim::IconHelper
module Decidim
module DecidimAwesome
module Voting
class ThreeFlagsCounterCell < Decidim::ViewModel
include Decidim::IconHelper

module DecidimAwesome
# ProposalVoteChannel handles the WebSocket connections for proposal votes.
# It streams from a channel specific to a proposal, identified by its ID.
class ProposalVoteChannel < ApplicationCable::Channel

Choose a reason for hiding this comment

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

I think we don't to use channels here.
Decidim is not using them and I would prefer not to introduce here unless is really necessary.

If you take a look at the file that is rendered by the VoteProposalController (update_buttons_and_counters.js.erb) it will replace the views automatically. I think it is best to not over-complicate it.

@microstudi microstudi merged commit fd679a0 into graded_proposal_voting Oct 27, 2023
10 of 11 checks passed
microstudi added a commit that referenced this pull request Nov 2, 2023
* Setup migrations and models (#13)

* add migrations

* change grade to weight

* totals name

* hooks (wip)

* add total weight cache

* graded voting/set defaults (#14)

* initial defaults

* add deface

* add views to manifest

* add cells view hacker

* set weight validator for voting manifest

* test fixing

* add basic weight voting

* add specs

* manifest spec

* add controller spec

* add readme

* fix awesome spec

* rubocop

* fix system checker spec

* set openstreet map

* used mocked openstreet maps

* modernize actions

* Prevent changin voting manifest if votes exist (#16)

* update codecov report

* Export weights (#17)

* add label generation for the manifest

* tweak exported

* add weights to the exporter

* fix tests

* add graphql entry

* Order proposals by my votes (#18)

* override filters

* add specs

* FrontEnd vote action UI/implementation (#15)

* add vote button, add modal, change votes counter view

* change frontend

* add voting to modal

* add action cable, updating results

* refactoring

* isolate the specific implementation of the voting

* add abstain setting

* change modal, change voting

* add abstain to voting

* refactoring

* change weight colors

* add tests

* add cells specs

* fix lint

* fix stylelint

* normalize locale

* change controller

* fix proposal_vote_path

* fix locale

* fix locale

---------

Co-authored-by: Ivan Vergés <[email protected]>

* Minor fixes and improvements for frontend voting (#19)

* change basic copies

* namespace css

* add link "change my vote"

* change opacity method

* add tests

* add check settings for abstain

* fix cell test

---------

Co-authored-by: Anna Topalidi <[email protected]>

* Add modal window with instructions (#20)

* change basic copies

* namespace css

* introduce modal pre-voting

* handle localstorage

* refactor namespaces for copies

* fix copy

* copies

* add finger

* add svg icons

* add margin

* fix margins in modal

* abstain style

* fix tests

* prevent voting abstain if not allowed

* fix footer

* normalize i18n

* add additional specs

* add conf var for sorting

* fix cache proposal_m

* proposal conf fixes

* Rename manifest and minor fixes (#22)

* rename voting_cards manifest

* mend

* abstain color

* fix modal title

* fix test

* fix counter colors

* fix xss

* style fix

---------

Co-authored-by: Anna Topalidi <[email protected]>
Co-authored-by: Anna Topalidi <[email protected]>
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