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

Feature/itemized tables #302

Merged
merged 45 commits into from
Jul 14, 2015
Merged

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Jun 26, 2015

Still in progress; not ready for merge!

jmcarp added a commit to jmcarp/openFEC-web-app that referenced this pull request Jul 2, 2015
@jmcarp
Copy link
Contributor Author

jmcarp commented Jul 13, 2015

Sounds like @msecret has some time to review. @noahmanger, would you prefer that we merge this without style updates once the code's in good shape, or do you want to hold off until everything looks nice? @msecret may also be able to help with styles.

@noahmanger
Copy link
Contributor

Ah yeah that's fine. I just noticed one thing that should be changed though: the "Filing Date" field should actually be "Transaction Date", referring to the date that the contribution was made, not the date of the report it was included in. Right, @LindsayYoung ?

@noahmanger
Copy link
Contributor

Also, I made decent work on style improvements. I'm fine to pass it off to someone else to finish up.

@noahmanger
Copy link
Contributor

I just went a head and filed a PR into @jmcarp 's branch with my /donations styling: jmcarp#3

@noahmanger
Copy link
Contributor

The next big task here is to implement the details pane in place of the current modal.

jmcarp added 5 commits July 13, 2015 17:03
Note: For this work as expected, we have to initialize tables (to expand
accordions) after initializing accordions, which is kind of brittle.
Open to alternatives @msecret.
Also use uniform badge styles, badges for develop and master.
@jmcarp jmcarp force-pushed the feature/itemized-tables branch from 06e5d76 to 4c1702d Compare July 13, 2015 21:21
<div class="modal__row">
<h3>Expenditure Detail</h3>
<nav class="modal__nav">
<div class="modal-content">
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be modal__content.

{{/if}}
</tr>
<tr>
<td>Recipient City & State</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

Same, &amp;.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handlebars renders this correctly as-is.

msecret pushed a commit that referenced this pull request Jul 14, 2015
@msecret msecret merged commit e64761f into fecgov:develop Jul 14, 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.

3 participants