-
Notifications
You must be signed in to change notification settings - Fork 14
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
Add 3 more sections to election summary page #388
Conversation
_includes/election-summary.html
Outdated
{% for source in election_totals.total_contributions_by_source %} | ||
{% assign amount = contribution_type[1] %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should the amount
be assigned to the source
variable that comes out of the for loop here? Or is this correct, in order to calculate/set the max
variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like both of these loops could be removed as the max value is now provided at the key total_contributions.
<h2>Who is contributing to races on this ballot?</h2> | ||
</div> | ||
<div class="l-section__content"> | ||
{% for contribution_type in election_totals.contributions_by_type %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above, you have this line written as:
{% for contribution_type in election_totals['contributions_by_type'] %}
Is there a reason for the difference?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No reason. I don't know why we wound up with a mix of square bracket and dot notation in this file. Dot would be consistent with the rest of the code, I think
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! I ❤️ consistency.
Couple more things:
|
Looks like it's supposed to be Candidate Name, Office but the office is missing from the data. The comma is left over from the dummy data, which included the office. Pinging @mikeubell here and on the backend.
Now that you point it out, I agree they look too big on mobile. They're the same size as candidate's data headings, like "Contributions", "Expenses", which are h2. Maybe they look big because they're long lines of text, instead of compact headings like "Expenses". How's h3 look? desquetopmobíl |
98e973b
to
396c2b2
Compare
396c2b2
to
f32d7d1
Compare
I like that better! The h2 compared to the size of the text in the side nav also just seemed awkward, maybe out of place? Good point about it being for longer phrases than just one word, I think the h3 is good. What do you think about changing the title of the page ("Data for Oakland Election...") to be an h2 instead of (I assume) the h1? It just seems so massive compared to everything else on the page. |
Partial resolution of #364
Adds
to the election summary page.
Does not add Top 5 Spenders. We're waiting for the data from the backend team for that one.
Previews
Large screens
Small screens