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

Implementing issue #49 #52

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Implementing issue #49 #52

wants to merge 2 commits into from

Conversation

jostrow
Copy link

@jostrow jostrow commented Apr 2, 2020

Initial commit for issue #49

DISCLAIMERS:
a) ./public/dataset/index.htm was not written in the React framework - it's just a standalone HTML file.
b) ./scripts/ directory was created, meant to house utilities/scripts that we use related to the website. This directory should not be public-facing. If you think those scripts belong somewhere else, that's fine, just let me know.

@@ -0,0 +1,182 @@
North American Mutual Aid Network Open Data Set
Copy link
Contributor

Choose a reason for hiding this comment

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

if you could convert this to markdown it would be great to include it in a docs/ folder in github and I can link it in the readme

Copy link
Author

Choose a reason for hiding this comment

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

@meganrm Okay, I have a .md version of the same file ready, plus removed one section which I shouldn't have included in either version.

Can you give me guidance on where the .md version should go? I have it sitting in this same folder, but this folder isn't meant to be public-facing. Are you planning to grab the .md file and put it somewhere else?

Copy link
Contributor

Choose a reason for hiding this comment

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

this page still seems to have funky quote marks

Copy link
Author

Choose a reason for hiding this comment

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

Oh, I didn't update the PR. Still getting used to how to do this...

Copy link
Author

Choose a reason for hiding this comment

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

@meganrm okay, figured it out. New commit is now posted with all the changes I described in these comments.

@meganrm
Copy link
Contributor

meganrm commented Apr 3, 2020

If you're feeling ambitious: The data is being read into memory on the front end, so to make this really useful, and have the download match the data online, it would be good to have it created on load. Here is a library for including a CSV download from a dataset on the front end. https://github.com/react-csv/react-csv. It could be included here: https://github.com/townhallproject/mutual-aid-networks/blob/master/src/components/NetworksTable/index.jsx#L123. Here is the schema for the database (although I need to add category and id to it. https://github.com/townhallproject/mutual-aid-networks/blob/master/src/components/NetworksTable/index.jsx#L123.

@@ -0,0 +1,241 @@
<!DOCTYPE html>
Copy link
Contributor

Choose a reason for hiding this comment

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

All the quotation marks in this are coming in malformed for me. They look like they're special characters.

While it doesn't hurt having this page here, looking it over, I think it would also be really useful to have it as the readme to the https://github.com/townhallproject/mutual-aid-hub-server, and we could include the csv over there too.

Copy link
Author

Choose a reason for hiding this comment

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

@meganrm Okay, funky quotes fixed. I thought about switching the HTML to indicate UTF-8 charset or similar, but opted to just replace the special character quotes with good ol' "s. I also removed the section of the license which doesn't belong (same thing I described in the other comment above).

If you'd like this content to live somewhere else, are you planning to move it around yourself, or can you tell me where it should go?

Finally, since it's been a week or so, I re-ran the script to generate a new version of the .csv dataset.

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