Skip to content

Ditch jQuery#26

Merged
waldyrious merged 9 commits intowaldyrious:gh-pagesfrom
Jan-Ka:gh-pages
Oct 27, 2017
Merged

Ditch jQuery#26
waldyrious merged 9 commits intowaldyrious:gh-pagesfrom
Jan-Ka:gh-pages

Conversation

@Jan-Ka
Copy link
Copy Markdown
Collaborator

@Jan-Ka Jan-Ka commented Oct 1, 2017

This will resolve #16 .

Commits of Interest / Discussion are:

  • c3ee841 adds .editorconfig
  • 0462e0d normalizes the codestyle
  • b1b0cee replaces flowtype with a non-jQuery version

The flowtype version is an adaptation from a Pull Request in the original Repository. I guess flowtype is no longer maintained.

Along the removal of jQuery are some fixes for issues that the original source had like some of the UI elements never getting hidden again or the script silently crashing when no result is returned.

@waldyrious
Copy link
Copy Markdown
Owner

Thanks for the contribution! I will do a proper review as soon as possible.

@waldyrious
Copy link
Copy Markdown
Owner

waldyrious commented Oct 27, 2017

@Jan-Ka many apologies for not getting back to you sooner. I went through the changes and they all look good to me (nice work on the separation of commits, by the way!).

I've tested this on http://rawgit.com/Jan-Ka/primerpedia/gh-pages/index.html (since directly loading https://jan-ka.github.io/primerpedia triggers mixed content errors and fails to load external resources), and apart from a minor change in vertical spacing, the result is pretty much as expected. Great work on handling the "not found" case!

Again, sorry for not reviewing earlier. I'll add you as a collaborator in case you'd like to play around some more :)

@waldyrious waldyrious merged commit 04677bf into waldyrious:gh-pages Oct 27, 2017
@Jan-Ka
Copy link
Copy Markdown
Collaborator Author

Jan-Ka commented Oct 28, 2017

Thank you very much @waldyrious ! Always glad to help!

waldyrious added a commit that referenced this pull request Oct 28, 2017
Line height was affected when the flowtype script was updated in #26, with the most visible effect being that the title of the article was not vertically centered in the blue box. This change fixes the line height for the time being, until issue #27 is implemented.
@waldyrious waldyrious mentioned this pull request Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants