Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Feature/election summary #610

Merged
merged 6 commits into from
Sep 10, 2015

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Sep 9, 2015

Add election summary data to homepage. Completes #591.

Branched off #605.
Depends on https://github.com/18F/openFEC/pull/1188.

@jmcarp jmcarp mentioned this pull request Sep 9, 2015
this.$disbursements = this.$elm.find('.disbursements');

this.fetch();
this.$elm.find('a.button--election').attr('href', this.buildUrl());
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the a in the selector actually needed?

I'd also suggest this should be a js-prefixed class rather then using a styling class, but will defer to you for best judgement.

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 think the a makes sense, since this behavior can only apply to anchor tags. Agreed re. js- prefixed classes; will update.

@jmcarp
Copy link
Contributor Author

jmcarp commented Sep 9, 2015

Updated!

@noahmanger
Copy link
Contributor

The "view all" link in the summary has the wrong URL "http://localhost:3000/president/2016/". Missing "/elections/"

@noahmanger
Copy link
Contributor

Other than that looks good to me whenever the api update gets merged. It's a little slow for me running the api locally, but hopefully that's helped when it's not local. If not, may be worth adding a loading animation or something. Or, like, a cool ticker effect.

@LindsayYoung
Copy link
Contributor

I think if we are showing candidate totals, we need independent expenditure totals for that race too.

@LindsayYoung
Copy link
Contributor

This looks good, I think we need to add independent expenditures to this, but that can be a separate pr.

LindsayYoung added a commit that referenced this pull request Sep 10, 2015
@LindsayYoung LindsayYoung merged commit a2ed993 into fecgov:develop Sep 10, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants