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

consistentfying #1159

Merged
merged 5 commits into from
Jul 17, 2017
Merged

consistentfying #1159

merged 5 commits into from
Jul 17, 2017

Conversation

brendansudol
Copy link
Contributor

fixes: https://github.com/18F/crime-data-frontend/issues/1030

a bunch of small consistent-fying

@@ -117,8 +117,7 @@ const NibrsContainer = ({
{content}
{isReady &&
<div className="serif italic fs-12">
Source: Reported {nibrsTerm} data from {placeDisplay},{' '}
{nibrsFirstYear}–{until}.
Source: Reported {nibrsTerm} data from {placeDisplay}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go the other way and include the year range in all sources

Copy link
Contributor Author

Choose a reason for hiding this comment

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

do you think it's important to have that in the source? I excluded so i didn't have to add that special nibrsFirstYear logic across files

Copy link
Contributor

Choose a reason for hiding this comment

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

Ya, I see your point. This reduces the complexity and the year range is always specified in the header. In the case of the NIBRS container it also contains a sentence about when the entity first started reporting NIBRS.

Jeremia Kimelman added 4 commits July 17, 2017 16:22
To be consistent with the other source texts (agency chart and NIBRS
chart), removing the dates from the trend chart source text. It remains
in the section's header and on the X axis
To make them more extensible (such as supplying a "size" prop or text)
I made the common terms in Terms.js full fledged react components.

I used children instead of a text prop to maintain consistency with how
the underlying Term component works
Remove book.svg because it wasn't being used for anything
@jeremiak jeremiak merged commit 2528d81 into master Jul 17, 2017
@jeremiak jeremiak deleted the bjs-data-source-consistent branch July 17, 2017 21:08
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.

2 participants