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

make the list of websites dynamic #2

Merged
merged 47 commits into from
Feb 22, 2021
Merged

make the list of websites dynamic #2

merged 47 commits into from
Feb 22, 2021

Conversation

MoralCode
Copy link
Contributor

@MoralCode MoralCode commented Feb 8, 2021

This is nearly done. Just needs:

  • Possibly a method of persisting the last known-good data to localstorage so if there is a breaking error, it defaults to saved data, rather than breaking (@Form-And-Function thoughts on this? it may introduce the risk of showing old data, but better than nothing???)
  • set up raygun monitoring to report errors as soon as the site breaks (such as in case someone messes up the airtable)
  • possibly a "misc" section in case we forget to fill in a column in the database or something.

This PR:

  • makes the list of sites dynamic/fetched from airtable so we can live-update it
  • replaces the home.html template with an HTML version of the page and merging it with the markdown version. There may be merge conflicts with this and Add basic map with covid vaccination sites #1, i can resolve these if needed
  • implements raygun monitoring so we are emailled when theres an error
  • adds a link to a google form for people to submit suggestions or corrections to our data (just the links)

@MoralCode MoralCode marked this pull request as ready for review February 8, 2021 21:12
@MoralCode MoralCode marked this pull request as draft February 8, 2021 21:13
@MoralCode MoralCode marked this pull request as ready for review February 9, 2021 05:28
@MoralCode
Copy link
Contributor Author

whoops, also gotta make it reusable first

@MoralCode MoralCode marked this pull request as draft February 9, 2021 06:35
@MoralCode MoralCode marked this pull request as ready for review February 14, 2021 20:30
@PrestonHager
Copy link
Contributor

Note on Jekyll: you can in fact embed the HTML inside the Markdown. So keep the old markdown file and then add your script file inside the Markdown file. I'd also keep it separate as its own file (for the script) for better readability.

@MoralCode
Copy link
Contributor Author

I'd also keep it separate as its own file (for the script) for better readability.

that's kinda what i did for the main/global script.js. ideally im trying to move as much of it as reasonable into helper functions inside that script file so the on-page scripts are pretty small. i think part of the reason for splitting it up and having a global script and an in-page script was because some of the in-page stuff needed to be done after the HTML elements loaded, although i dont see why we couldnt move the script.js's <script> line to the footer and have it check for the presence of elements with specific ID's and do everything needed to populate that specific one (maybe that would help loading times???)

This could also remove most of the HTML from the markdown. I haven't had a chance to research how jekyll handles the HTML-in-markdown scenario, but IIRC last time i tried something similar (markdown in front matter and using jekyll filters to convert it to HTML) it seemed to be including extra tags that could break our CSS if we end up doing anything super custom. Personally i think it might help make things easier to maintain (both for us and anyone using our code) if pages needing HTML had HTML extensions (or make everything .HTML, since jekyll lets you visit pages without the extension just fine)

Not a huge deal though, just some ideas surrounding your point.

Are you able to do a code review as well?

@MoralCode MoralCode mentioned this pull request Feb 15, 2021
@PrestonHager
Copy link
Contributor

I agree that we could make everything HTML files. It does seem a bit confusing to have both Markdown and HTML files, though Jekyll is compiled so either works fine for efficiency. Also, yes I can do a little bit of Code Review.

@MoralCode
Copy link
Contributor Author

also, do we want to keep the google form for suggesting changes or do we want to just say "email us"? it seems like email may be easier now since it lets us not have to remember to check a separate place for updates but also if we get a lot of updates it might create a lot of email traffic to keep up with

@PrestonHager
Copy link
Contributor

I think we should keep the email us, but we could also list the discord server and say the suggestions channel. But ultimately it's up to you since you are the one to check the email and whatnot.

Am I good to merge this now?

@MoralCode
Copy link
Contributor Author

Discord server is being added elsewhere in #5 and #7 i believe. if we have a specific channel for it, then yeah feel free to replace the google form with it if you want

Am I good to merge this now?

I was hoping this could get code-reviewed first as its a fairly major change. i also have a few breaking changes to the airtable DB i want to make first so ill go ahead and make those now before we deploy

@Form-And-Function Form-And-Function merged commit de8168c into main Feb 22, 2021
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.

3 participants