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

Analytics scripts, pie chart tweak #750

Merged
merged 8 commits into from
Jul 30, 2020

Conversation

adamkendis
Copy link
Member

@adamkendis adamkendis commented Jul 20, 2020

Fixes #745, fixes #746, fixes #747, fixes #748, fixes #749

  • Added Hotjar and Google optimizer scripts to index.html.
  • Updated comparison pie chart titles to display set name and set councils.
  • Added placeholder prop to MultiSelect SearchBar for custom placeholder text.
  • Changed NCSelector/CCSelector searchbar placeholder.

EDIT for additional commits:

  • Updated placeholder text for NCSelector and CCSelector and made text darker grey.

  • Changed header link text and removed color from Comparison link.

  • Removed the convoluted splashPage redux stuff. Added a /data route for the 311 Data Tool.

  • Removed trailing semicolon from list of selected request types in Comparison Criteria.

    • Up to date with dev branch
    • Branch name follows guidelines
    • All PR Status checks are successful
    • Peer reviewed and approved

Any questions? See the getting started guide

@adamkendis adamkendis requested a review from jmensch1 July 20, 2020 06:08
Copy link
Contributor

@jmensch1 jmensch1 left a comment

Choose a reason for hiding this comment

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

Looks good. Just to confirm, you wanted "Neighborhood" to be the placeholder for both the NCSelector and the CCSelector?

@@ -69,7 +69,7 @@ const Submit = ({
}
break;
}
default: return null;
default: return undefined;
Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to return undefined because react devtools was warning that effect functions shouldn't return null.

@adamkendis
Copy link
Member Author

Hey @jmensch1 , finally finished this up. Added a few more commits to include small changes discussed last meeting. New commit summary added in PR message.

@adamkendis adamkendis requested a review from jmensch1 July 30, 2020 18:23
@adamkendis adamkendis added the Do Not Merge Do not merge this PR, it is likely blocked by something else label Jul 30, 2020
@adamkendis
Copy link
Member Author

Hold off on merging. Resolving some route issues.

@adamkendis adamkendis removed the Do Not Merge Do not merge this PR, it is likely blocked by something else label Jul 30, 2020
@adamkendis
Copy link
Member Author

Good to go.

@jmensch1 jmensch1 merged commit 048bfab into hackforla:dev Jul 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants