Skip to content
This repository has been archived by the owner on Apr 15, 2019. It is now read-only.

Add selected votes counts bar - Closes #445 #754

Merged
merged 11 commits into from
Sep 26, 2017
Merged

Conversation

slaweet
Copy link
Contributor

@slaweet slaweet commented Sep 18, 2017

The goal is to add a bar fixed at the bottom of the voting page that displays numbers of votes selected for voting.

I had it previously implemented in Angular and it looked like this:
votes-bar
The original PR was #447

Closes #445

import style from './votingBar.css';

const VotingBar = ({ votes }) => {
const { maxCountOfVotes, maxCountOfVotesInOneTurn } = votingConst;
Copy link
Contributor

Choose a reason for hiding this comment

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

To prevent verbosity please perform this shorthand assignment while importing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wanted to use it like that, but it doesn't work.

import { maxCountOfVotes, maxCountOfVotesInOneTurn } from '../../constants/voting';

ends up in both maxCountOfVotes and maxCountOfVotesInOneTurn being undefined


const VotingBar = ({ votes }) => {
const { maxCountOfVotes, maxCountOfVotesInOneTurn } = votingConst;
const votedList = Object.keys(votes).filter(key => votes[key].confirmed);
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use a single forEach loop instead of 3 filters and file the lists thought the same iteration

Copy link
Contributor Author

@slaweet slaweet Sep 26, 2017

Choose a reason for hiding this comment

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

I set up a jsperf to see the difference:
https://jsperf.com/multiple-filters-vs-foreach

ForEach is 3 or 4 times faster, but the absolute values of how long it takes on a list of 100 still make the difference negligible.

I like using filters for their conciseness and readability.

const voteCount = Object.keys(votes).filter(
key => votes[key].confirmed !== votes[key].unconfirmed).length;
const currentVote = votes[action.data.username] || { unconfirmed: true, confirmed: false };
console.log(voteCount, votingConst.maxCountOfVotesInOneTurn, currentVote);
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove console.log. It's interesting that you have a log here and eslint doesn't fail

key => votes[key].confirmed !== votes[key].unconfirmed).length;
const currentVote = votes[action.data.username] || { unconfirmed: true, confirmed: false };
console.log(voteCount, votingConst.maxCountOfVotesInOneTurn, currentVote);
if (voteCount === votingConst.maxCountOfVotesInOneTurn + 1 &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be great if we show a toast for when we exceed 101 overall votes.

Copy link
Contributor

@reyraa reyraa left a comment

Choose a reason for hiding this comment

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

Thanks Vit.

@slaweet slaweet merged commit d62ef85 into development Sep 26, 2017
@slaweet slaweet deleted the 445-voting-bar branch September 26, 2017 12:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature request] Voting: Always visible total number of (un)votes and notification when reached 33 (un)votes
2 participants