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

Edit party list description in ElectionMapTooltip #220

Merged
merged 1 commit into from
Mar 25, 2019

Conversation

na399
Copy link
Contributor

@na399 na399 commented Mar 24, 2019

ref #214

From seat no.
Screenshot 2019-03-25 at 02 54 28,

to the number of seats of each party
Screenshot 2019-03-25 at 02 54 22.

NB there might be a faster way to query the numbers, instead of nationwidePartyStatsFromSummaryJSON(), that I'm unaware of.

@codeclimate
Copy link

codeclimate bot commented Mar 24, 2019

Code Climate has analyzed commit 738d575 and detected 0 issues on this pull request.

The test coverage on the diff in this pull request is 0.0% (0% is the threshold).

This pull request will bring the total coverage in the repository to 4.9% (-3.0% change).

View more on Code Climate.

@p16i
Copy link
Contributor

p16i commented Mar 25, 2019

Thanks for the PR. What do you think if we adjust the description slightly?

For example,

พรรคอนาคตใหม่
ได้ 56 ที่นั่ง

I'm going to merge anyway. This is a minor change.

@p16i p16i merged commit 0cf6718 into electinth:master Mar 25, 2019
@na399
Copy link
Contributor Author

na399 commented Mar 25, 2019

Thank you for the suggestion and the merge.

IMHO, I feel 'ได้' would be repetitive across the parties. Since it can be assumed that parties receive (ได้) the seats, I think the viewer should understand what the numbers mean and they can have faster access to the information (the number) without the need to read anything else, see Data-Ink ratio.

With the same reason, I was tempted to remove 'พรรค' in front of the party name, but I don't know whether that would go against the design proposal or not.

Anyway, if user research suggests otherwise, feel free to edit my commit krub. 😉

@p16i
Copy link
Contributor

p16i commented Mar 25, 2019

FY: the feature is on prod now.

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.

None yet

2 participants